On Mon, Jun 24, 2013 at 1:39 PM, Gurucharan Shetty <shet...@nicira.com>wrote:
> > > 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. > 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. >> > >
_______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
