On Thu, Aug 11, 2016 at 10:12:13AM -0500, Conner Herriges wrote:
> Ben Pfaff <b...@ovn.org> wrote on 08/10/2016 11:22:49 AM:
>
> > From: Ben Pfaff <b...@ovn.org>
> > To: Conner Herriges/US/IBM@IBM
> > Cc: dev@openvswitch.org
> > Date: 08/10/2016 11:23 AM
> > Subject: Re: [ovs-dev] [PATCH v2] Add monitoring to ovsdb
> >
> > On Wed, Aug 10, 2016 at 11:10:25AM -0500, Conner Herriges wrote:
> > > Monitoring is a valid option on POSIX based platforms. The monitor
> > > creates an additional process to monitor the ovsdb-server daemon. If
> > > the daemon dies due to a programming error, then the monitor process
> > > starts a new copy of it. If the daemon dies or exits for another
> > > reason, the monitor process exits.
> > >
> > > The monitor option is not currently enabled for either OVN Northboud or
> > > Southbound databases. It would be a trivial task to enable the monitor
> > > in the start_ovsdb function in ovn-ctl. However,
> > > this is an attempt to do so using the start_daemon function in ovs-lib,
> > > which is currently used by ovn-northd and ovn-controller.
> > >
> > > Signed-off-by: Conner Herriges <conner.herri...@ibm.com>
> >
> > Hi, this patch is space-damaged, notice "$DB_N B_ADDR" and similar.
> >
> > Also, please don't add a change log as comments in code.
> >
> > Why does this disable features for ovsdb-server in the start_daemon
> > function?
> >
> > Thanks,
> >
> > Ben.
>
> Hi Ben,
>
> I'm not exactly sure what you mean by adding a change log as comments
> in code. Can you clarify?
You added comments like this that say what you changed, rather than
what's happening:
#added start_daemon here
...
#added start_daemon here
...
#added ovsdb priority and wrapper here for consistency with
#northd and controller calls to start_daemon
When I'm reading code, I don't want to read a history of what's
changed. I want to read what the code is doing and why.
> The only feature that ovsdb-server requires in the start_daemon function
> is the --monitor. The rest of the features are enabled in ovn-ctl and are
> dependent on defaults and variables set there.
That's wrong. ovs-ctl uses start_daemon to start ovsdb-server and it
would break it to disable the features it uses.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
