[ovs-dev] [PATCH 2/2] ovsdb-server: Refactoring and clean up remote status reporting.

[Andy Zhou]

On Fri, Feb 26, 2016 at 4:39 PM, Ben Pfaff <blp at ovn.org> wrote:

> On Wed, Feb 24, 2016 at 01:27:34PM -0800, Andy Zhou wrote:
> > When reporting remote status, A listening remote will randomly
> > pick a session and report its session status. This does not seem
> > to make much sense. It is probably better to leave those fields
> > untouched.
> >
> > Update ovs-vswitchd.conf.db(5) to match the change in implementation.
> >
> > The man page says 'n_connections' should be at least 2. Make the
> > implementation match the description.
> >
> > Signed-off-by: Andy Zhou <azhou at ovn.org>
>
> It looks like changes in vswitch.xml use a 4-space indentation instead
> of the 2-space indentation used elsewhere there.  A few typos there,
> too: s/Both/both/, s/paris/pairs/, s/outbond/outbound/,
> s/inbond/inbound/.
>
> Thanks for the review. I will fix them in V2.


> I believe that ovsdb-server.c already implemented the semantics for
> n_connections > 1 in update_remote_row():
>
>     if (status.n_connections > 1) {
>         keys[n] = xstrdup("n_connections");
>         values[n++] = xasprintf("%d", status.n_connections);
>     }
>
> Based on that, I think it's best if
> ovsdb_jsonrpc_server_get_remote_status() just reports the raw number,
> e.g.:
>         status->n_connections = list_size(&remote->sessions);
>
> Sorry, I missed the check in update_remote_row().  will fix.


> The return value semantics described in the comment on
> ovsdb_jsonrpc_server_get_remote_status() seem wrong now.  I don't know
> whether that's important.
>

It would be nice to keep the comment accurate. I fixed the comment in v2
and the
implementation to match it.  Please let me know if you still see problems.

Thanks.