On Mon, Jun 24, 2013 at 1:10 PM, Ben Pfaff <b...@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 <gshe...@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. >
_______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
