On Thu, Jun 27, 2013 at 10:27 AM, Ben Pfaff <span dir="ltr"><<a href="mailto:blp@nicira.com" target="_blank">blp@nicira.com</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5">On Wed, Jun 26, 2013 at 11:29:36AM -0700, Gurucharan Shetty wrote:<br>
> On Wed, Jun 26, 2013 at 10:51 AM, Gurucharan Shetty <<a href="mailto:shettyg@nicira.com">shettyg@nicira.com</a>>wrote:<br>
><br>
> ><br>
> ><br>
> > On Tue, Jun 25, 2013 at 5:01 PM, Ben Pfaff <<a href="mailto:blp@nicira.com">blp@nicira.com</a>> wrote:<br>
> ><br>
> >> On Tue, Jun 25, 2013 at 01:18:36AM -0700, Gurucharan Shetty wrote:<br>
> >> > The commit allows a user to add a database file to a<br>
> >> > ovsdb-server during run time. One can also remove a<br>
> >> > database file from ovsdb-server's control.<br>
> >> ><br>
> >> > Feature #14595.<br>
> >> > Signed-off-by: Gurucharan Shetty <<a href="mailto:gshetty@nicira.com">gshetty@nicira.com</a>><br>
> >><br>
> >> I could see a number of ways to improve this. What do you think of my<br>
> >> version?<br>
> >><br>
> ><br>
> >> --8<--------------------------cut here-------------------------->8--<br>
> >><br>
> >> From: Ben Pfaff <<a href="mailto:blp@nicira.com">blp@nicira.com</a>><br>
> >> Date: Tue, 25 Jun 2013 17:00:56 -0700<br>
> >> Subject: [PATCH] ovsdb-server: Add and remove databases during run time.<br>
> >><br>
> >> The commit allows a user to add a database file to a<br>
> >> ovsdb-server during run time. One can also remove a<br>
> >> database file from ovsdb-server's control.<br>
> >><br>
> >> Feature #14595.<br>
> >> Signed-off-by: Gurucharan Shetty <<a href="mailto:gshetty@nicira.com">gshetty@nicira.com</a>><br>
> >> Signed-off-by: Ben Pfaff <<a href="mailto:blp@nicira.com">blp@nicira.com</a>><br>
> >><br>
> > I am fine with this. There is still a reference for reconfigure_from_db()<br>
> > in a comment.<br>
> > I pushed the first 2 patches of this series.<br>
> ><br>
> > (This commit will make add-db and remove-db needing different arguments.<br>
> > add-db needs<br>
> > file name and remove-db needs schema name. I don't have any strong<br>
> > feelings about it).<br>
> ><br>
> You will also need the following incremental.<br>
<br>
</div></div>Thanks. I folded that in as well as the following:<br>
<br>
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c<br>
index 016dd33..5f80502 100644<br>
--- a/ovsdb/jsonrpc-server.c<br>
+++ b/ovsdb/jsonrpc-server.c<br>
@@ -136,8 +136,16 @@ ovsdb_jsonrpc_server_add_db(struct ovsdb_jsonrpc_server *svr, struct ovsdb *db)<br>
<div class="im"> * true if successful, false if there is no database associated with 'db'. */<br>
</div> bool<br>
ovsdb_jsonrpc_server_remove_db(struct ovsdb_jsonrpc_server *svr,<br>
- struct ovsdb *db)<br>
+ struct ovsdb *db)<br>
{<br>
+ /* There might be pointers to 'db' from 'svr', such as monitors or<br>
+ * outstanding transactions. Disconnect all JSON-RPC connections to avoid<br>
+ * accesses to freed memory.<br>
+ *<br>
+ * If this is too big of a hammer in practice, we could be more selective,<br>
+ * e.g. disconnect only connections that actually reference 'db'. */<br>
+ ovsdb_jsonrpc_server_reconnect(svr);<br>
<div class="im">+<br>
return ovsdb_server_remove_db(&svr->up, db);<br>
}<br></div></blockquote><div>Thanks for doing this. I think we will also need to do the same after adding a DB. The following incremental does the job for me,</div><div><div>diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c</div>
<div>index 3520ffc..9847d80 100644</div><div>--- a/ovsdb/ovsdb-server.c</div><div>+++ b/ovsdb/ovsdb-server.c</div><div>@@ -1095,6 +1095,7 @@ ovsdb_server_add_database(struct unixctl_conn *conn, int argc OVS_UNU</div><div>
error = open_db(config, filename);</div><div> if (!error) {</div><div> save_config(config);</div><div>+ ovsdb_jsonrpc_server_reconnect(config->jsonrpc);</div><div> unixctl_command_reply(conn, NULL);</div>
<div> } else {</div><div> unixctl_command_reply_error(conn, error);</div></div><div><br></div><div>Everything else looks good to me.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">
<br>
</div>I'm appending the whole thing for a final round of review.<br>
<br>
--8<--------------------------cut here-------------------------->8--<br>
<br>
From: Gurucharan Shetty <<a href="mailto:gshetty@nicira.com">gshetty@nicira.com</a>><br>
Date: Thu, 27 Jun 2013 10:13:58 -0700<br>
<div class="im">Subject: [PATCH] ovsdb-server: Add and remove databases during run time.<br>
<br>
The commit allows a user to add a database file to a<br>
ovsdb-server during run time. One can also remove a<br>
database file from ovsdb-server's control.<br>
<br>
Feature #14595.<br>
Signed-off-by: Gurucharan Shetty <<a href="mailto:gshetty@nicira.com">gshetty@nicira.com</a>><br>
</div>Co-authored-by: Ben Pfaff <<a href="mailto:blp@nicira.com">blp@nicira.com</a>><br>
<div class="im">Signed-off-by: Ben Pfaff <<a href="mailto:blp@nicira.com">blp@nicira.com</a>><br>
---<br>
</div> ovsdb/jsonrpc-server.c | 17 ++<br>
<div class="im"> ovsdb/jsonrpc-server.h | 2 +<br>
ovsdb/<a href="http://ovsdb-server.1.in" target="_blank">ovsdb-server.1.in</a> | 23 +++<br>
</div> ovsdb/ovsdb-server.c | 389 ++++++++++++++++++++++++++++++++++-------------<br>
<div class="im"> ovsdb/server.c | 13 ++<br>
ovsdb/server.h | 1 +<br>
tests/<a href="http://ovsdb-server.at" target="_blank">ovsdb-server.at</a> | 121 +++++++++++++++<br>
</div> 7 files changed, 463 insertions(+), 103 deletions(-)<br>
<br>
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c<br>
index 9f99d64..5f80502 100644<br>
--- a/ovsdb/jsonrpc-server.c<br>
+++ b/ovsdb/jsonrpc-server.c<br>
@@ -132,6 +132,23 @@ ovsdb_jsonrpc_server_add_db(struct ovsdb_jsonrpc_server *svr, struct ovsdb *db)<br>
<div class="im"> return ovsdb_server_add_db(&svr->up, db);<br>
}<br>
<br>
+/* Removes 'db' from the set of databases served out by 'svr'. Returns<br>
+ * true if successful, false if there is no database associated with 'db'. */<br>
+bool<br>
+ovsdb_jsonrpc_server_remove_db(struct ovsdb_jsonrpc_server *svr,<br>
+ struct ovsdb *db)<br>
+{<br>
</div>+ /* There might be pointers to 'db' from 'svr', such as monitors or<br>
+ * outstanding transactions. Disconnect all JSON-RPC connections to avoid<br>
+ * accesses to freed memory.<br>
+ *<br>
+ * If this is too big of a hammer in practice, we could be more selective,<br>
+ * e.g. disconnect only connections that actually reference 'db'. */<br>
+ ovsdb_jsonrpc_server_reconnect(svr);<br>
<div><div class="h5">+<br>
+ return ovsdb_server_remove_db(&svr->up, db);<br>
+}<br>
+<br>
void<br>
ovsdb_jsonrpc_server_destroy(struct ovsdb_jsonrpc_server *svr)<br>
{<br>
diff --git a/ovsdb/jsonrpc-server.h b/ovsdb/jsonrpc-server.h<br>
index f2395fc..e6a1642 100644<br>
--- a/ovsdb/jsonrpc-server.h<br>
+++ b/ovsdb/jsonrpc-server.h<br>
@@ -26,6 +26,8 @@ struct simap;<br>
struct ovsdb_jsonrpc_server *ovsdb_jsonrpc_server_create(void);<br>
bool ovsdb_jsonrpc_server_add_db(struct ovsdb_jsonrpc_server *,<br>
struct ovsdb *);<br>
+bool ovsdb_jsonrpc_server_remove_db(struct ovsdb_jsonrpc_server *,<br>
+ struct ovsdb *);<br>
void ovsdb_jsonrpc_server_destroy(struct ovsdb_jsonrpc_server *);<br>
<br>
/* Options for a remote. */<br>
diff --git a/ovsdb/<a href="http://ovsdb-server.1.in" target="_blank">ovsdb-server.1.in</a> b/ovsdb/<a href="http://ovsdb-server.1.in" target="_blank">ovsdb-server.1.in</a><br>
index ceefef5..1201e6f 100644<br>
--- a/ovsdb/<a href="http://ovsdb-server.1.in" target="_blank">ovsdb-server.1.in</a><br>
+++ b/ovsdb/<a href="http://ovsdb-server.1.in" target="_blank">ovsdb-server.1.in</a><br>
@@ -150,6 +150,29 @@ not list remotes added indirectly because they were read from the<br>
database by configuring a<br>
\fBdb:\fIdb\fB,\fItable\fB,\fIcolumn\fR remote.<br>
.<br>
+.IP "\fBovsdb\-server/add\-db \fIdatabase\fR"<br>
+Adds the \fIdatabase\fR to the running \fBovsdb\-server\fR. The database<br>
+file must already have been created and initialized using, for example,<br>
+\fBovsdb\-tool create\fR.<br>
+.<br>
+.IP "\fBovsdb\-server/remove\-db \fIdatabase\fR"<br>
+Removes \fIdatabase\fR from the running \fBovsdb\-server\fR. \fIdatabase\fR<br>
+must be a database name as listed by \fBovsdb-server/list\-dbs\fR.<br>
+.IP<br>
+If a remote has been configured that points to the specified<br>
+\fIdatabase\fR (e.g. \fB\-\-remote=db:\fIdatabase\fB,\fR... on the<br>
+command line), then it will be disabled until another database with<br>
+the same name is added again (with \fBovsdb\-server/add\-db\fR).<br>
+.IP<br>
+Any public key infrastructure options specified through this database<br>
+(e.g. \fB\-\-private\-key=db:\fIdatabase,\fR... on the command line)<br>
+will be disabled until another database with the same name is added<br>
+again (with \fBovsdb\-server/add\-db\fR).<br>
+.<br>
+.IP "\fBovsdb\-server/list\-dbs"<br>
+Outputs a list of the currently configured databases added either through<br>
+the command line or through the \fBovsdb\-server/add\-db\fR command.<br>
+.<br>
.so lib/vlog-unixctl.man<br>
.so lib/memory-unixctl.man<br>
.so lib/coverage-unixctl.man<br>
diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c<br>
</div></div>index 919575a..3520ffc 100644<br>
<div><div class="h5">--- a/ovsdb/ovsdb-server.c<br>
+++ b/ovsdb/ovsdb-server.c<br>
@@ -77,38 +77,42 @@ static unixctl_cb_func ovsdb_server_exit;<br>
static unixctl_cb_func ovsdb_server_compact;<br>
static unixctl_cb_func ovsdb_server_reconnect;<br>
<br>
-struct add_remote_aux {<br>
+struct server_config {<br>
struct sset *remotes;<br>
struct shash *all_dbs;<br>
FILE *config_tmpfile;<br>
+ struct ovsdb_jsonrpc_server *jsonrpc;<br>
};<br>
static unixctl_cb_func ovsdb_server_add_remote;<br>
-<br>
-struct remove_remote_aux {<br>
- struct sset *remotes;<br>
- FILE *config_tmpfile;<br>
-};<br>
static unixctl_cb_func ovsdb_server_remove_remote;<br>
static unixctl_cb_func ovsdb_server_list_remotes;<br>
<br>
-static void open_db(struct ovsdb_jsonrpc_server *jsonrpc,<br>
- struct db *db, struct shash *all_dbs);<br>
+static unixctl_cb_func ovsdb_server_add_database;<br>
+static unixctl_cb_func ovsdb_server_remove_database;<br>
+static unixctl_cb_func ovsdb_server_list_databases;<br>
+<br>
+static char *open_db(struct server_config *config, const char *filename);<br>
<br>
static void parse_options(int *argc, char **argvp[],<br>
struct sset *remotes, char **unixctl_pathp,<br>
char **run_command);<br>
static void usage(void) NO_RETURN;<br>
<br>
-static void reconfigure_from_db(struct ovsdb_jsonrpc_server *jsonrpc,<br>
- const struct shash *all_dbs,<br>
- struct sset *remotes);<br>
+static char *reconfigure_remotes(struct ovsdb_jsonrpc_server *,<br>
+ const struct shash *all_dbs,<br>
+ struct sset *remotes);<br>
+static char *reconfigure_ssl(const struct shash *all_dbs);<br>
+static void report_error_if_changed(char *error, char **last_errorp);<br>
<br>
static void update_remote_status(const struct ovsdb_jsonrpc_server *jsonrpc,<br>
const struct sset *remotes,<br>
struct shash *all_dbs);<br>
<br>
-static void save_config(FILE *config_file, const struct sset *);<br>
-static void load_config(FILE *config_file, struct sset *);<br>
+static void save_config__(FILE *config_file, const struct sset *remotes,<br>
+ const struct sset *db_filenames);<br>
+static void save_config(struct server_config *);<br>
+static void load_config(FILE *config_file, struct sset *remotes,<br>
+ struct sset *db_filenames);<br>
<br>
int<br>
main(int argc, char *argv[])<br>
</div></div>@@ -117,17 +121,18 @@ main(int argc, char *argv[])<br>
<div class="im"> char *run_command = NULL;<br>
struct unixctl_server *unixctl;<br>
struct ovsdb_jsonrpc_server *jsonrpc;<br>
- struct sset remotes;<br>
+ struct sset remotes, db_filenames;<br>
+ const char *db_filename;<br>
struct process *run_process;<br>
bool exiting;<br>
int retval;<br>
long long int status_timer = LLONG_MIN;<br>
- struct add_remote_aux add_remote_aux;<br>
- struct remove_remote_aux remove_remote_aux;<br>
FILE *config_tmpfile;<br>
-<br>
+ struct server_config server_config;<br>
</div><div class="im"> struct shash all_dbs;<br>
struct shash_node *node;<br>
+ char *remotes_error, *ssl_error;<br>
+ char *error;<br>
int i;<br>
<br>
proctitle_init(argc, argv);<br>
</div>@@ -147,29 +152,46 @@ main(int argc, char *argv[])<br>
<div><div class="h5"> if (!config_tmpfile) {<br>
ovs_fatal(errno, "failed to create temporary file");<br>
}<br>
- save_config(config_tmpfile, &remotes);<br>
+<br>
+ sset_init(&db_filenames);<br>
+ if (argc > 0) {<br>
+ for (i = 0; i < argc; i++) {<br>
+ sset_add(&db_filenames, argv[i]);<br>
+ }<br>
+ } else {<br>
+ char *default_db = xasprintf("%s/conf.db", ovs_dbdir());<br>
+ sset_add(&db_filenames, default_db);<br>
+ free(default_db);<br>
+ }<br>
+<br>
+ server_config.remotes = &remotes;<br>
+ server_config.config_tmpfile = config_tmpfile;<br>
+<br>
+ save_config__(config_tmpfile, &remotes, &db_filenames);<br>
<br>
daemonize_start();<br>
<br>
/* Load the saved config. */<br>
- load_config(config_tmpfile, &remotes);<br>
-<br>
- shash_init(&all_dbs);<br>
+ load_config(config_tmpfile, &remotes, &db_filenames);<br>
jsonrpc = ovsdb_jsonrpc_server_create();<br>
<br>
- if (argc > 0) {<br>
- for (i = 0; i < argc; i++) {<br>
</div></div>- struct db *db = xzalloc(sizeof *db);<br>
<div class="im">- db->filename = argv[i];<br>
- open_db(jsonrpc, db, &all_dbs);<br>
- }<br>
- } else {<br>
</div>- struct db *db = xzalloc(sizeof *db);<br>
<div class="im">- db->filename = xasprintf("%s/conf.db", ovs_dbdir());<br>
- open_db(jsonrpc, db, &all_dbs);<br>
+ shash_init(&all_dbs);<br>
+ server_config.all_dbs = &all_dbs;<br>
+ server_config.jsonrpc = jsonrpc;<br>
+ SSET_FOR_EACH (db_filename, &db_filenames) {<br>
+ error = open_db(&server_config, db_filename);<br>
+ if (error) {<br>
+ ovs_fatal(0, "%s", error);<br>
+ }<br>
}<br>
<br>
- reconfigure_from_db(jsonrpc, &all_dbs, &remotes);<br>
+ error = reconfigure_remotes(jsonrpc, &all_dbs, &remotes);<br>
+ if (!error) {<br>
+ error = reconfigure_ssl(&all_dbs);<br>
+ }<br>
+ if (error) {<br>
+ ovs_fatal(0, "%s", error);<br>
+ }<br>
<br>
retval = unixctl_server_create(unixctl_path, &unixctl);<br>
if (retval) {<br>
</div>@@ -207,21 +229,23 @@ main(int argc, char *argv[])<br>
<div><div class="h5"> unixctl_command_register("ovsdb-server/reconnect", "", 0, 0,<br>
ovsdb_server_reconnect, jsonrpc);<br>
<br>
- add_remote_aux.remotes = &remotes;<br>
- add_remote_aux.all_dbs = &all_dbs;<br>
- add_remote_aux.config_tmpfile = config_tmpfile;<br>
unixctl_command_register("ovsdb-server/add-remote", "REMOTE", 1, 1,<br>
- ovsdb_server_add_remote, &add_remote_aux);<br>
-<br>
- remove_remote_aux.remotes = &remotes;<br>
- remove_remote_aux.config_tmpfile = config_tmpfile;<br>
+ ovsdb_server_add_remote, &server_config);<br>
unixctl_command_register("ovsdb-server/remove-remote", "REMOTE", 1, 1,<br>
- ovsdb_server_remove_remote, &remove_remote_aux);<br>
-<br>
+ ovsdb_server_remove_remote, &server_config);<br>
unixctl_command_register("ovsdb-server/list-remotes", "", 0, 0,<br>
ovsdb_server_list_remotes, &remotes);<br>
<br>
+ unixctl_command_register("ovsdb-server/add-db", "DB", 1, 1,<br>
+ ovsdb_server_add_database, &server_config);<br>
+ unixctl_command_register("ovsdb-server/remove-db", "DB", 1, 1,<br>
+ ovsdb_server_remove_database, &server_config);<br>
+ unixctl_command_register("ovsdb-server/list-dbs", "", 0, 0,<br>
+ ovsdb_server_list_databases, &all_dbs);<br>
+<br>
exiting = false;<br>
+ ssl_error = NULL;<br>
+ remotes_error = NULL;<br>
while (!exiting) {<br>
memory_run();<br>
if (memory_should_report()) {<br>
</div></div>@@ -237,12 +261,15 @@ main(int argc, char *argv[])<br>
simap_destroy(&usage);<br>
}<br>
<br>
- /* Run unixctl_server_run() before reconfigure_from_db() because<br>
+ /* Run unixctl_server_run() before reconfigure_remotes() because<br>
* ovsdb-server/add-remote and ovsdb-server/remove-remote can change<br>
- * the set of remotes that reconfigure_from_db() uses. */<br>
+ * the set of remotes that reconfigure_remotes() uses. */<br>
<div class="im"> unixctl_server_run(unixctl);<br>
<br>
- reconfigure_from_db(jsonrpc, &all_dbs, &remotes);<br>
+ report_error_if_changed(<br>
+ reconfigure_remotes(jsonrpc, &all_dbs, &remotes),<br>
+ &remotes_error);<br>
+ report_error_if_changed(reconfigure_ssl(&all_dbs), &ssl_error);<br>
ovsdb_jsonrpc_server_run(jsonrpc);<br>
<br>
SHASH_FOR_EACH(node, &all_dbs) {<br>
</div>@@ -297,24 +324,31 @@ main(int argc, char *argv[])<br>
<div><div class="h5"> return 0;<br>
}<br>
<br>
-static void<br>
-open_db(struct ovsdb_jsonrpc_server *jsonrpc, struct db *db,<br>
- struct shash *all_dbs)<br>
+static char *<br>
+open_db(struct server_config *config, const char *filename)<br>
{<br>
- struct ovsdb_error *error;<br>
+ struct ovsdb_error *db_error;<br>
+ struct db *db;<br>
+ char *error;<br>
<br>
- error = ovsdb_file_open(db->filename, false,<br>
- &db->db, &db->file);<br>
- if (error) {<br>
- ovs_fatal(0, "%s", ovsdb_error_to_string(error));<br>
- }<br>
+ db = xzalloc(sizeof *db);<br>
+ db->filename = xstrdup(filename);<br>
<br>
- if (!ovsdb_jsonrpc_server_add_db(jsonrpc, db->db)) {<br>
- ovs_fatal(0, "%s: duplicate database name",<br>
- db->db->schema->name);<br>
+ db_error = ovsdb_file_open(db->filename, false, &db->db, &db->file);<br>
+ if (db_error) {<br>
+ error = ovsdb_error_to_string(db_error);<br>
+ } else if (!ovsdb_jsonrpc_server_add_db(config->jsonrpc, db->db)) {<br>
+ error = xasprintf("%s: duplicate database name", db->db->schema->name);<br>
+ } else {<br>
+ shash_add_assert(config->all_dbs, db->db->schema->name, db);<br>
+ return NULL;<br>
}<br>
<br>
- shash_add(all_dbs, db->filename, db);<br>
+ ovsdb_error_destroy(db_error);<br>
+ ovsdb_destroy(db->db);<br>
+ free(db->filename);<br>
+ free(db);<br>
+ return error;<br>
}<br>
<br>
static const struct db *<br>
</div></div>@@ -426,8 +460,9 @@ parse_db_string_column(const struct shash *all_dbs,<br>
<div class="im"> return NULL;<br>
}<br>
<br>
-static OVS_UNUSED const char *<br>
-query_db_string(const struct shash *all_dbs, const char *name)<br>
+static const char *<br>
+query_db_string(const struct shash *all_dbs, const char *name,<br>
+ struct ds *errors)<br>
{<br>
if (!name || strncmp(name, "db:", 3)) {<br>
return name;<br>
</div>@@ -441,7 +476,8 @@ query_db_string(const struct shash *all_dbs, const char *name)<br>
<div class="im"> retval = parse_db_string_column(all_dbs, name,<br>
&db, &table, &column);<br>
if (retval) {<br>
- ovs_fatal(0, "%s", retval);<br>
+ ds_put_format(errors, "%s\n", retval);<br>
+ return NULL;<br>
}<br>
<br>
HMAP_FOR_EACH (row, hmap_node, &table->rows) {<br>
</div>@@ -666,7 +702,7 @@ add_manager_options(struct shash *remotes, const struct ovsdb_row *row)<br>
<div class="im"><br>
static void<br>
query_db_remotes(const char *name, const struct shash *all_dbs,<br>
- struct shash *remotes)<br>
+ struct shash *remotes, struct ds *errors)<br>
{<br>
const struct ovsdb_column *column;<br>
const struct ovsdb_table *table;<br>
</div>@@ -676,7 +712,9 @@ query_db_remotes(const char *name, const struct shash *all_dbs,<br>
<div class="im"><br>
retval = parse_db_column(all_dbs, name, &db, &table, &column);<br>
if (retval) {<br>
- ovs_fatal(0, "%s", retval);<br>
+ ds_put_format(errors, "%s\n", retval);<br>
+ free(retval);<br>
+ return;<br>
}<br>
<br>
if (column->type.key.type == OVSDB_TYPE_STRING<br>
</div>@@ -791,7 +829,8 @@ update_remote_rows(const struct shash *all_dbs,<br>
<div class="im"><br>
retval = parse_db_column(all_dbs, remote_name, &db, &table, &column);<br>
if (retval) {<br>
- ovs_fatal(0, "%s", retval);<br>
+ free(retval);<br>
+ return;<br>
}<br>
<br>
if (column->type.key.type != OVSDB_TYPE_UUID<br>
</div>@@ -850,11 +889,12 @@ update_remote_status(const struct ovsdb_jsonrpc_server *jsonrpc,<br>
<div class="im"> }<br>
}<br>
<br>
-/* Reconfigures ovsdb-server based on information in the database. */<br>
-static void<br>
-reconfigure_from_db(struct ovsdb_jsonrpc_server *jsonrpc,<br>
+/* Reconfigures ovsdb-server's remotes based on information in the database. */<br>
+static char *<br>
+reconfigure_remotes(struct ovsdb_jsonrpc_server *jsonrpc,<br>
const struct shash *all_dbs, struct sset *remotes)<br>
{<br>
+ struct ds errors = DS_EMPTY_INITIALIZER;<br>
struct shash resolved_remotes;<br>
const char *name;<br>
<br>
</div>@@ -862,7 +902,7 @@ reconfigure_from_db(struct ovsdb_jsonrpc_server *jsonrpc,<br>
<div class="im"> shash_init(&resolved_remotes);<br>
SSET_FOR_EACH (name, remotes) {<br>
if (!strncmp(name, "db:", 3)) {<br>
- query_db_remotes(name, all_dbs, &resolved_remotes);<br>
+ query_db_remotes(name, all_dbs, &resolved_remotes, &errors);<br>
} else {<br>
add_remote(&resolved_remotes, name);<br>
}<br>
</div>@@ -870,11 +910,42 @@ reconfigure_from_db(struct ovsdb_jsonrpc_server *jsonrpc,<br>
<div><div class="h5"> ovsdb_jsonrpc_server_set_remotes(jsonrpc, &resolved_remotes);<br>
shash_destroy_free_data(&resolved_remotes);<br>
<br>
- /* Configure SSL. */<br>
- stream_ssl_set_key_and_cert(query_db_string(all_dbs, private_key_file),<br>
- query_db_string(all_dbs, certificate_file));<br>
- stream_ssl_set_ca_cert_file(query_db_string(all_dbs, ca_cert_file),<br>
- bootstrap_ca_cert);<br>
+ return errors.string;<br>
+}<br>
+<br>
+static char *<br>
+reconfigure_ssl(const struct shash *all_dbs)<br>
+{<br>
+ struct ds errors = DS_EMPTY_INITIALIZER;<br>
+ const char *resolved_private_key;<br>
+ const char *resolved_certificate;<br>
+ const char *resolved_ca_cert;<br>
+<br>
+ resolved_private_key = query_db_string(all_dbs, private_key_file, &errors);<br>
+ resolved_certificate = query_db_string(all_dbs, certificate_file, &errors);<br>
+ resolved_ca_cert = query_db_string(all_dbs, ca_cert_file, &errors);<br>
+<br>
+ stream_ssl_set_key_and_cert(resolved_private_key, resolved_certificate);<br>
+ stream_ssl_set_ca_cert_file(resolved_ca_cert, bootstrap_ca_cert);<br>
+<br>
+ return errors.string;<br>
+}<br>
+<br>
+static void<br>
+report_error_if_changed(char *error, char **last_errorp)<br>
+{<br>
+ if (error) {<br>
+ if (!*last_errorp || strcmp(error, *last_errorp)) {<br>
+ VLOG_WARN("%s", error);<br>
+ free(*last_errorp);<br>
+ *last_errorp = error;<br>
+ return;<br>
+ }<br>
+ free(error);<br>
+ } else {<br>
+ free(*last_errorp);<br>
+ *last_errorp = NULL;<br>
+ }<br>
}<br>
<br>
static void<br>
</div></div>@@ -946,9 +1017,9 @@ ovsdb_server_reconnect(struct unixctl_conn *conn, int argc OVS_UNUSED,<br>
<div class="im"> * ovsdb-server services. */<br>
static void<br>
ovsdb_server_add_remote(struct unixctl_conn *conn, int argc OVS_UNUSED,<br>
- const char *argv[], void *aux_)<br>
+ const char *argv[], void *config_)<br>
{<br>
- struct add_remote_aux *aux = aux_;<br>
+ struct server_config *config = config_;<br>
const char *remote = argv[1];<br>
<br>
const struct ovsdb_column *column;<br>
</div>@@ -958,11 +1029,11 @@ ovsdb_server_add_remote(struct unixctl_conn *conn, int argc OVS_UNUSED,<br>
<div class="im"><br>
retval = (strncmp("db:", remote, 3)<br>
? NULL<br>
- : parse_db_column(aux->all_dbs, remote,<br>
+ : parse_db_column(config->all_dbs, remote,<br>
&db, &table, &column));<br>
if (!retval) {<br>
- if (sset_add(aux->remotes, remote)) {<br>
- save_config(aux->config_tmpfile, aux->remotes);<br>
+ if (sset_add(config->remotes, remote)) {<br>
+ save_config(config);<br>
}<br>
unixctl_command_reply(conn, NULL);<br>
} else {<br>
</div>@@ -975,15 +1046,15 @@ ovsdb_server_add_remote(struct unixctl_conn *conn, int argc OVS_UNUSED,<br>
<div class="im"> * that ovsdb-server services. */<br>
static void<br>
ovsdb_server_remove_remote(struct unixctl_conn *conn, int argc OVS_UNUSED,<br>
- const char *argv[], void *aux_)<br>
+ const char *argv[], void *config_)<br>
{<br>
- struct remove_remote_aux *aux = aux_;<br>
+ struct server_config *config = config_;<br>
struct sset_node *node;<br>
<br>
- node = sset_find(aux->remotes, argv[1]);<br>
+ node = sset_find(config->remotes, argv[1]);<br>
if (node) {<br>
- sset_delete(aux->remotes, node);<br>
- save_config(aux->config_tmpfile, aux->remotes);<br>
+ sset_delete(config->remotes, node);<br>
+ save_config(config);<br>
unixctl_command_reply(conn, NULL);<br>
} else {<br>
unixctl_command_reply_error(conn, "no such remote");<br>
</div>@@ -1011,6 +1082,76 @@ ovsdb_server_list_remotes(struct unixctl_conn *conn, int argc OVS_UNUSED,<br>
<div><div class="h5"> ds_destroy(&s);<br>
}<br>
<br>
+<br>
+/* "ovsdb-server/add-db DB": adds the DB to ovsdb-server. */<br>
+static void<br>
+ovsdb_server_add_database(struct unixctl_conn *conn, int argc OVS_UNUSED,<br>
+ const char *argv[], void *config_)<br>
+{<br>
+ struct server_config *config = config_;<br>
+ const char *filename = argv[1];<br>
+ char *error;<br>
+<br>
+ error = open_db(config, filename);<br>
+ if (!error) {<br>
+ save_config(config);<br>
+ unixctl_command_reply(conn, NULL);<br>
+ } else {<br>
+ unixctl_command_reply_error(conn, error);<br>
+ free(error);<br>
+ }<br>
+}<br>
+<br>
+static void<br>
+ovsdb_server_remove_database(struct unixctl_conn *conn, int argc OVS_UNUSED,<br>
+ const char *argv[], void *config_)<br>
+{<br>
+ struct server_config *config = config_;<br>
+ struct shash_node *node;<br>
</div></div><div class="im">+ struct db *db;<br>
+ bool ok;<br>
+<br>
+ node = shash_find(config->all_dbs, argv[1]);<br>
+ if (!node) {<br>
+ unixctl_command_reply_error(conn, "Failed to find the database.");<br>
+ return;<br>
+ }<br>
+ db = node->data;<br>
+<br>
+ ok = ovsdb_jsonrpc_server_remove_db(config->jsonrpc, db->db);<br>
+ ovs_assert(ok);<br>
+<br>
</div>+ ovsdb_destroy(db->db);<br>
+ shash_delete(config->all_dbs, node);<br>
<div class="im">+ free(db->filename);<br>
+ free(db);<br>
+<br>
</div><div><div class="h5">+ save_config(config);<br>
+ unixctl_command_reply(conn, NULL);<br>
+}<br>
+<br>
+static void<br>
+ovsdb_server_list_databases(struct unixctl_conn *conn, int argc OVS_UNUSED,<br>
+ const char *argv[] OVS_UNUSED, void *all_dbs_)<br>
+{<br>
+ struct shash *all_dbs = all_dbs_;<br>
+ const struct shash_node **nodes;<br>
+ struct ds s;<br>
+ size_t i;<br>
+<br>
+ ds_init(&s);<br>
+<br>
+ nodes = shash_sort(all_dbs);<br>
+ for (i = 0; i < shash_count(all_dbs); i++) {<br>
+ struct db *db = nodes[i]->data;<br>
+ ds_put_format(&s, "%s\n", db->db->schema->name);<br>
+ }<br>
+ free(nodes);<br>
+<br>
+ unixctl_command_reply(conn, ds_cstr(&s));<br>
+ ds_destroy(&s);<br>
+}<br>
+<br>
static void<br>
parse_options(int *argcp, char **argvp[],<br>
struct sset *remotes, char **unixctl_pathp, char **run_command)<br>
</div></div>@@ -1131,25 +1272,37 @@ usage(void)<br>
<div><div class="h5"> exit(EXIT_SUCCESS);<br>
}<br>
<br>
-/* Truncates and replaces the contents of 'config_file' by a representation<br>
- * of 'remotes'. */<br>
+static struct json *<br>
+sset_to_json(const struct sset *sset)<br>
+{<br>
+ struct json *array;<br>
+ const char *s;<br>
+<br>
+ array = json_array_create_empty();<br>
+ SSET_FOR_EACH (s, sset) {<br>
+ json_array_add(array, json_string_create(s));<br>
+ }<br>
+ return array;<br>
+}<br>
+<br>
+/* Truncates and replaces the contents of 'config_file' by a representation of<br>
+ * 'remotes' and 'db_filenames'. */<br>
static void<br>
-save_config(FILE *config_file, const struct sset *remotes)<br>
+save_config__(FILE *config_file, const struct sset *remotes,<br>
+ const struct sset *db_filenames)<br>
{<br>
- const char *remote;<br>
- struct json *json;<br>
+ struct json *obj;<br>
char *s;<br>
<br>
if (ftruncate(fileno(config_file), 0) == -1) {<br>
VLOG_FATAL("failed to truncate temporary file (%s)", strerror(errno));<br>
}<br>
<br>
- json = json_array_create_empty();<br>
- SSET_FOR_EACH (remote, remotes) {<br>
- json_array_add(json, json_string_create(remote));<br>
- }<br>
- s = json_to_string(json, 0);<br>
- json_destroy(json);<br>
+ obj = json_object_create();<br>
+ json_object_put(obj, "remotes", sset_to_json(remotes));<br>
+ json_object_put(obj, "db_filenames", sset_to_json(db_filenames));<br>
+ s = json_to_string(obj, 0);<br>
+ json_destroy(obj);<br>
<br>
if (fseek(config_file, 0, SEEK_SET) != 0<br>
|| fputs(s, config_file) == EOF<br>
</div></div>@@ -1159,15 +1312,45 @@ save_config(FILE *config_file, const struct sset *remotes)<br>
<div><div class="h5"> free(s);<br>
}<br>
<br>
-/* Clears and replaces 'remotes' by a configuration read from 'config_file',<br>
- * which must have been previously written by save_config(). */<br>
+/* Truncates and replaces the contents of 'config_file' by a representation of<br>
+ * 'config'. */<br>
static void<br>
-load_config(FILE *config_file, struct sset *remotes)<br>
+save_config(struct server_config *config)<br>
+{<br>
+ struct sset db_filenames;<br>
+ struct shash_node *node;<br>
+<br>
+ sset_init(&db_filenames);<br>
+ SHASH_FOR_EACH (node, config->all_dbs) {<br>
+ struct db *db = node->data;<br>
+ sset_add(&db_filenames, db->filename);<br>
+ }<br>
+<br>
+ save_config__(config->config_tmpfile, config->remotes, &db_filenames);<br>
+<br>
+ sset_destroy(&db_filenames);<br>
+}<br>
+<br>
+static void<br>
+sset_from_json(struct sset *sset, const struct json *array)<br>
{<br>
- struct json *json;<br>
size_t i;<br>
<br>
- sset_clear(remotes);<br>
+ sset_clear(sset);<br>
+<br>
+ ovs_assert(array->type == JSON_ARRAY);<br>
+ for (i = 0; i < array->u.array.n; i++) {<br>
+ const struct json *elem = array->u.array.elems[i];<br>
+ sset_add(sset, json_string(elem));<br>
+ }<br>
+}<br>
+<br>
+/* Clears and replaces 'remotes' and 'dbnames' by a configuration read from<br>
+ * 'config_file', which must have been previously written by save_config(). */<br>
+static void<br>
+load_config(FILE *config_file, struct sset *remotes, struct sset *db_filenames)<br>
+{<br>
+ struct json *json;<br>
<br>
if (fseek(config_file, 0, SEEK_SET) != 0) {<br>
VLOG_FATAL("seek failed in temporary file (%s)", strerror(errno));<br>
</div></div>@@ -1176,10 +1359,10 @@ load_config(FILE *config_file, struct sset *remotes)<br>
<div class="HOEnZb"><div class="h5"> if (json->type == JSON_STRING) {<br>
VLOG_FATAL("reading json failed (%s)", json_string(json));<br>
}<br>
- ovs_assert(json->type == JSON_ARRAY);<br>
- for (i = 0; i < json->u.array.n; i++) {<br>
- const struct json *remote = json->u.array.elems[i];<br>
- sset_add(remotes, json_string(remote));<br>
- }<br>
+ ovs_assert(json->type == JSON_OBJECT);<br>
+<br>
+ sset_from_json(remotes, shash_find_data(json_object(json), "remotes"));<br>
+ sset_from_json(db_filenames,<br>
+ shash_find_data(json_object(json), "db_filenames"));<br>
json_destroy(json);<br>
}<br>
diff --git a/ovsdb/server.c b/ovsdb/server.c<br>
index bf4ef3c..82f55cb 100644<br>
--- a/ovsdb/server.c<br>
+++ b/ovsdb/server.c<br>
@@ -132,6 +132,19 @@ ovsdb_server_add_db(struct ovsdb_server *server, struct ovsdb *db)<br>
return shash_add_once(&server->dbs, db->schema->name, db);<br>
}<br>
<br>
+/* Removes 'db' from the set of databases served out by 'server'. Returns<br>
+ * true if successful, false if there is no db associated with<br>
+ * db->schema->name. */<br>
+bool<br>
+ovsdb_server_remove_db(struct ovsdb_server *server, struct ovsdb *db)<br>
+{<br>
+ void *data = shash_find_and_delete(&server->dbs, db->schema->name);<br>
+ if (data) {<br>
+ return true;<br>
+ }<br>
+ return false;<br>
+}<br>
+<br>
/* Destroys 'server'. */<br>
void<br>
ovsdb_server_destroy(struct ovsdb_server *server)<br>
diff --git a/ovsdb/server.h b/ovsdb/server.h<br>
index 561f01e..047cbb7 100644<br>
--- a/ovsdb/server.h<br>
+++ b/ovsdb/server.h<br>
@@ -83,6 +83,7 @@ struct ovsdb_server {<br>
<br>
void ovsdb_server_init(struct ovsdb_server *);<br>
bool ovsdb_server_add_db(struct ovsdb_server *, struct ovsdb *);<br>
+bool ovsdb_server_remove_db(struct ovsdb_server *, struct ovsdb *);<br>
void ovsdb_server_destroy(struct ovsdb_server *);<br>
<br>
struct ovsdb_lock_waiter *ovsdb_server_lock(struct ovsdb_server *,<br>
diff --git a/tests/<a href="http://ovsdb-server.at" target="_blank">ovsdb-server.at</a> b/tests/<a href="http://ovsdb-server.at" target="_blank">ovsdb-server.at</a><br>
index 2368cbc..16a1d95 100644<br>
--- a/tests/<a href="http://ovsdb-server.at" target="_blank">ovsdb-server.at</a><br>
+++ b/tests/<a href="http://ovsdb-server.at" target="_blank">ovsdb-server.at</a><br>
@@ -164,6 +164,127 @@ AT_CHECK(<br>
OVSDB_SERVER_SHUTDOWN<br>
AT_CLEANUP<br>
<br>
+AT_SETUP([ovsdb-server/add-db and remove-db])<br>
+AT_KEYWORDS([ovsdb server positive])<br>
+ON_EXIT([kill `cat ovsdb-server.pid`])<br>
+OVS_RUNDIR=`pwd`; export OVS_RUNDIR<br>
+OVS_LOGDIR=`pwd`; export OVS_LOGDIR<br>
+ordinal_schema > schema1<br>
+constraint_schema > schema2<br>
+AT_CHECK([ovsdb-tool create db1 schema1], [0], [ignore], [ignore])<br>
+AT_CHECK([ovsdb-tool create db2 schema2], [0], [ignore], [ignore])<br>
+<br>
+# Start ovsdb-server with just a single database - db1.<br>
+AT_CHECK([ovsdb-server --detach --no-chdir --pidfile --remote=punix:socket db1], [0])<br>
+AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/list-dbs],<br>
+ [0], [ordinals<br>
+])<br>
+<br>
+# Add the second database.<br>
+AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/add-db db2], [0])<br>
+AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/list-dbs],<br>
+ [0], [constraints<br>
+ordinals<br>
+])<br>
+<br>
+# The databases are responsive.<br>
+AT_CHECK([ovsdb-client list-tables unix:socket constraints], [0], [ignore], [ignore])<br>
+AT_CHECK([ovsdb-client list-tables unix:socket ordinals], [0], [ignore], [ignore])<br>
+<br>
+# Add an already added database.<br>
+AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/add-db db2], 2, [], [stderr])<br>
+AT_CHECK([sed 's/(.*)/(...)/' stderr], [0],<br>
+ [I/O error: db2: failed to lock lockfile (...)<br>
+ovs-appctl: ovsdb-server: server returned an error<br>
+])<br>
+<br>
+# Add a non-existing database.<br>
+AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/add-db db3], 2, [], [stderr])<br>
+AT_CHECK([sed 's/(.*)/(...)/' stderr], [0],<br>
+ [I/O error: open: db3 failed (...)<br>
+ovs-appctl: ovsdb-server: server returned an error<br>
+])<br>
+<br>
+# Add a remote through a db path in db1.<br>
+AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/add-remote db:ordinals,ordinals,name], [0])<br>
+AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/list-remotes],<br>
+ [0], [db:ordinals,ordinals,name<br>
+punix:socket<br>
+])<br>
+<br>
+# Removing db1 has no effect on its remote.<br>
+AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/remove-db ordinals], [0])<br>
+AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/list-dbs],<br>
+ [0], [constraints<br>
+])<br>
+AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/list-remotes],<br>
+ [0], [db:ordinals,ordinals,name<br>
+punix:socket<br>
+])<br>
+AT_CHECK([ovsdb-client list-tables unix:socket ordinals], [1], [ignore], [ignore])<br>
+<br>
+# Remove db2.<br>
+AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/remove-db constraints], [0])<br>
+AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/list-dbs],<br>
+ [0], [])<br>
+AT_CHECK([ovsdb-client list-tables unix:socket constraints], [1], [ignore], [ignore])<br>
+<br>
+# Remove a non-existent database.<br>
+AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/remove-db ordinals], [2],<br>
+ [], [Failed to find the database.<br>
+ovs-appctl: ovsdb-server: server returned an error<br>
+])<br>
+AT_CLEANUP<br>
+<br>
+AT_SETUP([ovsdb-server/add-db and remove-db with --monitor])<br>
+AT_KEYWORDS([ovsdb server positive])<br>
+# Start ovsdb-server, initially with one db.<br>
+OVS_RUNDIR=`pwd`; export OVS_RUNDIR<br>
+OVS_LOGDIR=`pwd`; export OVS_LOGDIR<br>
+ordinal_schema > schema<br>
+AT_CHECK([ovsdb-tool create db1 schema], [0], [ignore], [ignore])<br>
+ON_EXIT([kill `cat *.pid`])<br>
+AT_CHECK([ovsdb-server -v -vvlog:off --monitor --detach --no-chdir --pidfile --log-file db1])<br>
+<br>
+# Add the second database.<br>
+constraint_schema > schema2<br>
+AT_CHECK([ovsdb-tool create db2 schema2], [0], [ignore], [ignore])<br>
+AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/add-db db2], [0])<br>
+AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/list-dbs],<br>
+ [0], [constraints<br>
+ordinals<br>
+])<br>
+<br>
+# Kill the daemon process, making it look like a segfault,<br>
+# and wait for a new daemon process to get spawned.<br>
+cp ovsdb-server.pid old.pid<br>
+AT_CHECK([kill -SEGV `cat ovsdb-server.pid`])<br>
+OVS_WAIT_WHILE([kill -0 `cat old.pid`])<br>
+OVS_WAIT_UNTIL(<br>
+ [test -s ovsdb-server.pid && test `cat ovsdb-server.pid` != `cat old.pid`])<br>
+AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/list-dbs],<br>
+ [0], [constraints<br>
+ordinals<br>
+])<br>
+<br>
+# Remove the recently added database.<br>
+AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/remove-db constraints])<br>
+AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/list-dbs],<br>
+ [0], [ordinals<br>
+])<br>
+<br>
+# Kill the daemon process, making it look like a segfault,<br>
+# and wait for a new daemon process to get spawned.<br>
+cp ovsdb-server.pid old.pid<br>
+AT_CHECK([kill -SEGV `cat ovsdb-server.pid`])<br>
+OVS_WAIT_WHILE([kill -0 `cat old.pid`])<br>
+OVS_WAIT_UNTIL(<br>
+ [test -s ovsdb-server.pid && test `cat ovsdb-server.pid` != `cat old.pid`])<br>
+AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/list-dbs],<br>
+ [0], [ordinals<br>
+])<br>
+AT_CLEANUP<br>
+<br>
AT_SETUP([--remote=db: implementation])<br>
AT_KEYWORDS([ovsdb server positive])<br>
OVS_RUNDIR=`pwd`; export OVS_RUNDIR<br>
--<br>
1.7.2.5<br>
<br>
</div></div></blockquote></div><br>