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

Gurucharan Shetty shettyg at nicira.com
Mon Jun 24 20:39:15 UTC 2013


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>
>
> The documentation could use a bit more detail.  I think that readers
> might not know where to look, otherwise.  How about this:
>
> ----------------------------------------------------------------------
> .IP "\fBovsdb\-server/add\-db \fIdatabase\fR"
> Adds \fIdatabase\fR (a file name) to the running
> \fBovsdb\-server\fR. The database file must already have been created
> and initialized using, for example, \fBovsdb\-tool create\fR.
> .
> .IP "\fBovsdb\-server/remove\-db \fIdatabase\fR"
> Removes \fIdatabase\fR from the running \fBovsdb\-server\fR.
> \fIdatabase\fR must be a database name as listed by
> \fBovsdb-server/list\-dbs\fR.
> .IP
> If a remote has been configured that points to the specified
> \fIdatabase\fR (e.g. \fB\-\-remote=db:\fIdatabase\fB,\fR... on the
> command line), then it is automatically removed.
> .IP
> Any public key infrastructure options specified through this database
> (e.g. \fB\-\-private\-key=db:\fIdatabase,\fR... on the command line)
> are no longer checked for new changes, but any files previously
> configured are still used. (Adding \fIdatabase\fR back will cause the
> public key infrastructure options specified through it to update
> again.)
> .
> .IP "\fBovsdb\-server/list\-dbs"
> Outputs a list of the currently configured databases added either through
> the command line or through the \fBovsdb\-server/add\-db\fR command.
>
Okay.


> ----------------------------------------------------------------------
>
> 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.
Do you feel this is more preferable? When a second database is added, say
through a script, they won't get any error. Will we be okay if the main
Open_vSwitch database stops working because of it?

Same with SSL.


> The return value for ovsdb_server_remove_db() seems odd.  I would make
> it return a bool.
>
Okay.

>
> This change adds some lines ending in \ to
> ovsdb_server_add_database().  That's odd.
>
I removed it.

>
> ssl_path_valid() has a function call wrapped across lines, that
> doesn't need to be.
>
Okay.

>
> Thanks,
>
> Ben.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20130624/57be999f/attachment-0003.html>


More information about the dev mailing list