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

Ben Pfaff blp at nicira.com
Mon Jun 24 21:30:21 UTC 2013


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.



More information about the dev mailing list