[ovs-dev] [PATCH 2/2] ovsdb-server: Add and remove databases during run time.

Gurucharan Shetty shettyg at nicira.com
Tue Jun 25 16:47:16 UTC 2013


On Mon, Jun 24, 2013 at 2:30 PM, Ben Pfaff <blp at nicira.com> wrote:

> On Mon, Jun 24, 2013 at 01:39:15PM -0700, Gurucharan Shetty wrote:
> > On Mon, Jun 24, 2013 at 1:10 PM, Ben Pfaff <blp at nicira.com> wrote:
> >
> > > On Thu, Jun 20, 2013 at 07:38:48AM -0700, Gurucharan Shetty wrote:
> > > > The commit allows a user to add a database file to a
> > > > ovsdb-server during run time. One can also remove a
> > > > database file from ovsdb-server's control.
> > > >
> > > > Feature #14595.
> > > > Signed-off-by: Gurucharan Shetty <gshetty at nicira.com>
> >
> > > This commit seems to use the high-level approach of "figure out
> > > whether something will succeed, then actually do it only if it will
> > > succeed," when the thing that it actually wants to do will exit with a
> > > fatal error if it fails.  I think that it would be safer to instead
> > > modify the code that exits with a fatal error to return an error to
> > > the caller to handle.  With this approach, there is no chance that the
> > > code to figure out whether the action will succeed will be wrong, and
> > > as the code evolves later the two pieces of code cannot get out of
> > > sync.  What do you think?
> > >
> > I think I understand what you are saying. But consider the following
> case.
> >
> > case 1:
> > We start with just one database. And we add a remote that points to a DB
> > column without a schema. ex: Open_vSwitch,manager_options.
> > When I add a new database during runtime, the above remote is
> > invalid(because of a missing schema). If I go ahead and add the database,
> > and change the code such that in the main loop, I throw an error, then it
> > will only go to the log files and there will be one error during every
> run.
>
> I don't mean to change the user-visible behavior from what you already
> have here, only the implementation.
>
> Here's an example of what I mean.  The new code tries to guess in
> advance with ssl_path_valid() whether the next configuration will
> succeed, and then it either does or doesn't use that in each of the
> next runs based on that guess.  The guess is supposed to be correct,
> but it's always hard to tell whether that's exactly right.
>
> I'd suggest that, instead, one would modify reconfigure_from_db() and
> the functions that it calls, as necessary, to return an error to the
> caller if the configuration can't work.  The initial
> reconfigure_from_db() call at startup would abort with a fatal error
> if reconfigure_from_db() failed.  The ones during the main loop would
> log an error (probably only each time the error string changes)
> instead.
>
Right now, with a single database, we can provide a DB path without the db
name.
This patch would check for its validity when someone adds the second DB. I
sent a V2
which makes the DB name mandatory and got rid of those checks here.

Also, for SSL paths, I moved it into reconfigure_from_db and only raise the
error once.
During startup, if the DB paths are invalid, we still exit (as before).

Thanks,
Guru
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20130625/8cfdda78/attachment-0003.html>


More information about the dev mailing list