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. ---------------------------------------------------------------------- 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? The return value for ovsdb_server_remove_db() seems odd. I would make it return a bool. This change adds some lines ending in \ to ovsdb_server_add_database(). That's odd. ssl_path_valid() has a function call wrapped across lines, that doesn't need to be. Thanks, Ben. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
