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

Gurucharan Shetty shettyg at nicira.com
Mon Jun 24 20:44:59 UTC 2013


On Mon, Jun 24, 2013 at 1:39 PM, Gurucharan Shetty <shettyg at nicira.com>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>
>>
>> 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.
>
Another option is to go ahead and add the database but invalidate the wrong
remote and SSL config. With this, we will thrown one error in log messages.

>
>
>> 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/077a6853/attachment-0003.html>


More information about the dev mailing list