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

Ben Pfaff blp at nicira.com
Thu Jun 27 17:27:57 UTC 2013


On Wed, Jun 26, 2013 at 11:29:36AM -0700, Gurucharan Shetty wrote:
> On Wed, Jun 26, 2013 at 10:51 AM, Gurucharan Shetty <shettyg at nicira.com>wrote:
> 
> >
> >
> > On Tue, Jun 25, 2013 at 5:01 PM, Ben Pfaff <blp at nicira.com> wrote:
> >
> >> On Tue, Jun 25, 2013 at 01:18:36AM -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>
> >>
> >> I could see a number of ways to improve this.  What do you think of my
> >> version?
> >>
> >
> >> --8<--------------------------cut here-------------------------->8--
> >>
> >> From: Ben Pfaff <blp at nicira.com>
> >> Date: Tue, 25 Jun 2013 17:00:56 -0700
> >> Subject: [PATCH] ovsdb-server: Add and remove databases during run time.
> >>
> >> 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>
> >> Signed-off-by: Ben Pfaff <blp at nicira.com>
> >>
> > I am fine with this. There is still a reference for reconfigure_from_db()
> > in a comment.
> > I pushed the first 2 patches of this series.
> >
> > (This commit will make add-db and remove-db needing different arguments.
> > add-db needs
> > file name and remove-db needs schema name. I don't have any strong
> > feelings about it).
> >
> You will also need the following incremental.

Thanks.  I folded that in as well as the following:

diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index 016dd33..5f80502 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -136,8 +136,16 @@ ovsdb_jsonrpc_server_add_db(struct ovsdb_jsonrpc_server *svr, struct ovsdb *db)
  * true if successful, false if there is no database associated with 'db'. */
 bool
 ovsdb_jsonrpc_server_remove_db(struct ovsdb_jsonrpc_server *svr,
-                                struct ovsdb *db)
+                               struct ovsdb *db)
 {
+    /* There might be pointers to 'db' from 'svr', such as monitors or
+     * outstanding transactions.  Disconnect all JSON-RPC connections to avoid
+     * accesses to freed memory.
+     *
+     * If this is too big of a hammer in practice, we could be more selective,
+     * e.g. disconnect only connections that actually reference 'db'. */
+    ovsdb_jsonrpc_server_reconnect(svr);
+
     return ovsdb_server_remove_db(&svr->up, db);
 }

I'm appending the whole thing for a final round of review.

--8<--------------------------cut here-------------------------->8--

From: Gurucharan Shetty <gshetty at nicira.com>
Date: Thu, 27 Jun 2013 10:13:58 -0700
Subject: [PATCH] ovsdb-server: Add and remove databases during run time.

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>
Co-authored-by: Ben Pfaff <blp at nicira.com>
Signed-off-by: Ben Pfaff <blp at nicira.com>
---
 ovsdb/jsonrpc-server.c  |   17 ++
 ovsdb/jsonrpc-server.h  |    2 +
 ovsdb/ovsdb-server.1.in |   23 +++
 ovsdb/ovsdb-server.c    |  389 ++++++++++++++++++++++++++++++++++-------------
 ovsdb/server.c          |   13 ++
 ovsdb/server.h          |    1 +
 tests/ovsdb-server.at   |  121 +++++++++++++++
 7 files changed, 463 insertions(+), 103 deletions(-)

diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index 9f99d64..5f80502 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -132,6 +132,23 @@ ovsdb_jsonrpc_server_add_db(struct ovsdb_jsonrpc_server *svr, struct ovsdb *db)
     return ovsdb_server_add_db(&svr->up, db);
 }
 
+/* Removes 'db' from the set of databases served out by 'svr'.  Returns
+ * true if successful, false if there is no database associated with 'db'. */
+bool
+ovsdb_jsonrpc_server_remove_db(struct ovsdb_jsonrpc_server *svr,
+                               struct ovsdb *db)
+{
+    /* There might be pointers to 'db' from 'svr', such as monitors or
+     * outstanding transactions.  Disconnect all JSON-RPC connections to avoid
+     * accesses to freed memory.
+     *
+     * If this is too big of a hammer in practice, we could be more selective,
+     * e.g. disconnect only connections that actually reference 'db'. */
+    ovsdb_jsonrpc_server_reconnect(svr);
+
+    return ovsdb_server_remove_db(&svr->up, db);
+}
+
 void
 ovsdb_jsonrpc_server_destroy(struct ovsdb_jsonrpc_server *svr)
 {
diff --git a/ovsdb/jsonrpc-server.h b/ovsdb/jsonrpc-server.h
index f2395fc..e6a1642 100644
--- a/ovsdb/jsonrpc-server.h
+++ b/ovsdb/jsonrpc-server.h
@@ -26,6 +26,8 @@ struct simap;
 struct ovsdb_jsonrpc_server *ovsdb_jsonrpc_server_create(void);
 bool ovsdb_jsonrpc_server_add_db(struct ovsdb_jsonrpc_server *,
                                  struct ovsdb *);
+bool ovsdb_jsonrpc_server_remove_db(struct ovsdb_jsonrpc_server *,
+                                     struct ovsdb *);
 void ovsdb_jsonrpc_server_destroy(struct ovsdb_jsonrpc_server *);
 
 /* Options for a remote. */
diff --git a/ovsdb/ovsdb-server.1.in b/ovsdb/ovsdb-server.1.in
index ceefef5..1201e6f 100644
--- a/ovsdb/ovsdb-server.1.in
+++ b/ovsdb/ovsdb-server.1.in
@@ -150,6 +150,29 @@ not list remotes added indirectly because they were read from the
 database by configuring a
 \fBdb:\fIdb\fB,\fItable\fB,\fIcolumn\fR remote.
 .
+.IP "\fBovsdb\-server/add\-db \fIdatabase\fR"
+Adds the \fIdatabase\fR 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 will be disabled until another database with
+the same name is added again (with \fBovsdb\-server/add\-db\fR).
+.IP
+Any public key infrastructure options specified through this database
+(e.g. \fB\-\-private\-key=db:\fIdatabase,\fR... on the command line)
+will be disabled until another database with the same name is added
+again (with \fBovsdb\-server/add\-db\fR).
+.
+.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.
+.
 .so lib/vlog-unixctl.man
 .so lib/memory-unixctl.man
 .so lib/coverage-unixctl.man
diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
index 919575a..3520ffc 100644
--- a/ovsdb/ovsdb-server.c
+++ b/ovsdb/ovsdb-server.c
@@ -77,38 +77,42 @@ static unixctl_cb_func ovsdb_server_exit;
 static unixctl_cb_func ovsdb_server_compact;
 static unixctl_cb_func ovsdb_server_reconnect;
 
-struct add_remote_aux {
+struct server_config {
     struct sset *remotes;
     struct shash *all_dbs;
     FILE *config_tmpfile;
+    struct ovsdb_jsonrpc_server *jsonrpc;
 };
 static unixctl_cb_func ovsdb_server_add_remote;
-
-struct remove_remote_aux {
-    struct sset *remotes;
-    FILE *config_tmpfile;
-};
 static unixctl_cb_func ovsdb_server_remove_remote;
 static unixctl_cb_func ovsdb_server_list_remotes;
 
-static void open_db(struct ovsdb_jsonrpc_server *jsonrpc,
-                    struct db *db, struct shash *all_dbs);
+static unixctl_cb_func ovsdb_server_add_database;
+static unixctl_cb_func ovsdb_server_remove_database;
+static unixctl_cb_func ovsdb_server_list_databases;
+
+static char *open_db(struct server_config *config, const char *filename);
 
 static void parse_options(int *argc, char **argvp[],
                           struct sset *remotes, char **unixctl_pathp,
                           char **run_command);
 static void usage(void) NO_RETURN;
 
-static void reconfigure_from_db(struct ovsdb_jsonrpc_server *jsonrpc,
-                                const struct shash *all_dbs,
-                                struct sset *remotes);
+static char *reconfigure_remotes(struct ovsdb_jsonrpc_server *,
+                                 const struct shash *all_dbs,
+                                 struct sset *remotes);
+static char *reconfigure_ssl(const struct shash *all_dbs);
+static void report_error_if_changed(char *error, char **last_errorp);
 
 static void update_remote_status(const struct ovsdb_jsonrpc_server *jsonrpc,
                                  const struct sset *remotes,
                                  struct shash *all_dbs);
 
-static void save_config(FILE *config_file, const struct sset *);
-static void load_config(FILE *config_file, struct sset *);
+static void save_config__(FILE *config_file, const struct sset *remotes,
+                          const struct sset *db_filenames);
+static void save_config(struct server_config *);
+static void load_config(FILE *config_file, struct sset *remotes,
+                        struct sset *db_filenames);
 
 int
 main(int argc, char *argv[])
@@ -117,17 +121,18 @@ main(int argc, char *argv[])
     char *run_command = NULL;
     struct unixctl_server *unixctl;
     struct ovsdb_jsonrpc_server *jsonrpc;
-    struct sset remotes;
+    struct sset remotes, db_filenames;
+    const char *db_filename;
     struct process *run_process;
     bool exiting;
     int retval;
     long long int status_timer = LLONG_MIN;
-    struct add_remote_aux add_remote_aux;
-    struct remove_remote_aux remove_remote_aux;
     FILE *config_tmpfile;
-
+    struct server_config server_config;
     struct shash all_dbs;
     struct shash_node *node;
+    char *remotes_error, *ssl_error;
+    char *error;
     int i;
 
     proctitle_init(argc, argv);
@@ -147,29 +152,46 @@ main(int argc, char *argv[])
     if (!config_tmpfile) {
         ovs_fatal(errno, "failed to create temporary file");
     }
-    save_config(config_tmpfile, &remotes);
+
+    sset_init(&db_filenames);
+    if (argc > 0) {
+        for (i = 0; i < argc; i++) {
+            sset_add(&db_filenames, argv[i]);
+         }
+    } else {
+        char *default_db = xasprintf("%s/conf.db", ovs_dbdir());
+        sset_add(&db_filenames, default_db);
+        free(default_db);
+    }
+
+    server_config.remotes = &remotes;
+    server_config.config_tmpfile = config_tmpfile;
+
+    save_config__(config_tmpfile, &remotes, &db_filenames);
 
     daemonize_start();
 
     /* Load the saved config. */
-    load_config(config_tmpfile, &remotes);
-
-    shash_init(&all_dbs);
+    load_config(config_tmpfile, &remotes, &db_filenames);
     jsonrpc = ovsdb_jsonrpc_server_create();
 
-    if (argc > 0) {
-        for (i = 0; i < argc; i++) {
-            struct db *db = xzalloc(sizeof *db);
-            db->filename = argv[i];
-            open_db(jsonrpc, db, &all_dbs);
-         }
-    } else {
-        struct db *db = xzalloc(sizeof *db);
-        db->filename = xasprintf("%s/conf.db", ovs_dbdir());
-        open_db(jsonrpc, db, &all_dbs);
+    shash_init(&all_dbs);
+    server_config.all_dbs = &all_dbs;
+    server_config.jsonrpc = jsonrpc;
+    SSET_FOR_EACH (db_filename, &db_filenames) {
+        error = open_db(&server_config, db_filename);
+        if (error) {
+            ovs_fatal(0, "%s", error);
+        }
     }
 
-    reconfigure_from_db(jsonrpc, &all_dbs, &remotes);
+    error = reconfigure_remotes(jsonrpc, &all_dbs, &remotes);
+    if (!error) {
+        error = reconfigure_ssl(&all_dbs);
+    }
+    if (error) {
+        ovs_fatal(0, "%s", error);
+    }
 
     retval = unixctl_server_create(unixctl_path, &unixctl);
     if (retval) {
@@ -207,21 +229,23 @@ main(int argc, char *argv[])
     unixctl_command_register("ovsdb-server/reconnect", "", 0, 0,
                              ovsdb_server_reconnect, jsonrpc);
 
-    add_remote_aux.remotes = &remotes;
-    add_remote_aux.all_dbs = &all_dbs;
-    add_remote_aux.config_tmpfile = config_tmpfile;
     unixctl_command_register("ovsdb-server/add-remote", "REMOTE", 1, 1,
-                             ovsdb_server_add_remote, &add_remote_aux);
-
-    remove_remote_aux.remotes = &remotes;
-    remove_remote_aux.config_tmpfile = config_tmpfile;
+                             ovsdb_server_add_remote, &server_config);
     unixctl_command_register("ovsdb-server/remove-remote", "REMOTE", 1, 1,
-                             ovsdb_server_remove_remote, &remove_remote_aux);
-
+                             ovsdb_server_remove_remote, &server_config);
     unixctl_command_register("ovsdb-server/list-remotes", "", 0, 0,
                              ovsdb_server_list_remotes, &remotes);
 
+    unixctl_command_register("ovsdb-server/add-db", "DB", 1, 1,
+                             ovsdb_server_add_database, &server_config);
+    unixctl_command_register("ovsdb-server/remove-db", "DB", 1, 1,
+                             ovsdb_server_remove_database, &server_config);
+    unixctl_command_register("ovsdb-server/list-dbs", "", 0, 0,
+                             ovsdb_server_list_databases, &all_dbs);
+
     exiting = false;
+    ssl_error = NULL;
+    remotes_error = NULL;
     while (!exiting) {
         memory_run();
         if (memory_should_report()) {
@@ -237,12 +261,15 @@ main(int argc, char *argv[])
             simap_destroy(&usage);
         }
 
-        /* Run unixctl_server_run() before reconfigure_from_db() because
+        /* Run unixctl_server_run() before reconfigure_remotes() because
          * ovsdb-server/add-remote and ovsdb-server/remove-remote can change
-         * the set of remotes that reconfigure_from_db() uses. */
+         * the set of remotes that reconfigure_remotes() uses. */
         unixctl_server_run(unixctl);
 
-        reconfigure_from_db(jsonrpc, &all_dbs, &remotes);
+        report_error_if_changed(
+            reconfigure_remotes(jsonrpc, &all_dbs, &remotes),
+            &remotes_error);
+        report_error_if_changed(reconfigure_ssl(&all_dbs), &ssl_error);
         ovsdb_jsonrpc_server_run(jsonrpc);
 
         SHASH_FOR_EACH(node, &all_dbs) {
@@ -297,24 +324,31 @@ main(int argc, char *argv[])
     return 0;
 }
 
-static void
-open_db(struct ovsdb_jsonrpc_server *jsonrpc, struct db *db,
-        struct shash *all_dbs)
+static char *
+open_db(struct server_config *config, const char *filename)
 {
-    struct ovsdb_error *error;
+    struct ovsdb_error *db_error;
+    struct db *db;
+    char *error;
 
-    error = ovsdb_file_open(db->filename, false,
-                            &db->db, &db->file);
-    if (error) {
-        ovs_fatal(0, "%s", ovsdb_error_to_string(error));
-    }
+    db = xzalloc(sizeof *db);
+    db->filename = xstrdup(filename);
 
-    if (!ovsdb_jsonrpc_server_add_db(jsonrpc, db->db)) {
-        ovs_fatal(0, "%s: duplicate database name",
-                  db->db->schema->name);
+    db_error = ovsdb_file_open(db->filename, false, &db->db, &db->file);
+    if (db_error) {
+        error = ovsdb_error_to_string(db_error);
+    } else if (!ovsdb_jsonrpc_server_add_db(config->jsonrpc, db->db)) {
+        error = xasprintf("%s: duplicate database name", db->db->schema->name);
+    } else {
+        shash_add_assert(config->all_dbs, db->db->schema->name, db);
+        return NULL;
     }
 
-    shash_add(all_dbs, db->filename, db);
+    ovsdb_error_destroy(db_error);
+    ovsdb_destroy(db->db);
+    free(db->filename);
+    free(db);
+    return error;
 }
 
 static const struct db *
@@ -426,8 +460,9 @@ parse_db_string_column(const struct shash *all_dbs,
     return NULL;
 }
 
-static OVS_UNUSED const char *
-query_db_string(const struct shash *all_dbs, const char *name)
+static const char *
+query_db_string(const struct shash *all_dbs, const char *name,
+                struct ds *errors)
 {
     if (!name || strncmp(name, "db:", 3)) {
         return name;
@@ -441,7 +476,8 @@ query_db_string(const struct shash *all_dbs, const char *name)
         retval = parse_db_string_column(all_dbs, name,
                                         &db, &table, &column);
         if (retval) {
-            ovs_fatal(0, "%s", retval);
+            ds_put_format(errors, "%s\n", retval);
+            return NULL;
         }
 
         HMAP_FOR_EACH (row, hmap_node, &table->rows) {
@@ -666,7 +702,7 @@ add_manager_options(struct shash *remotes, const struct ovsdb_row *row)
 
 static void
 query_db_remotes(const char *name, const struct shash *all_dbs,
-                 struct shash *remotes)
+                 struct shash *remotes, struct ds *errors)
 {
     const struct ovsdb_column *column;
     const struct ovsdb_table *table;
@@ -676,7 +712,9 @@ query_db_remotes(const char *name, const struct shash *all_dbs,
 
     retval = parse_db_column(all_dbs, name, &db, &table, &column);
     if (retval) {
-        ovs_fatal(0, "%s", retval);
+        ds_put_format(errors, "%s\n", retval);
+        free(retval);
+        return;
     }
 
     if (column->type.key.type == OVSDB_TYPE_STRING
@@ -791,7 +829,8 @@ update_remote_rows(const struct shash *all_dbs,
 
     retval = parse_db_column(all_dbs, remote_name, &db, &table, &column);
     if (retval) {
-        ovs_fatal(0, "%s", retval);
+        free(retval);
+        return;
     }
 
     if (column->type.key.type != OVSDB_TYPE_UUID
@@ -850,11 +889,12 @@ update_remote_status(const struct ovsdb_jsonrpc_server *jsonrpc,
     }
 }
 
-/* Reconfigures ovsdb-server based on information in the database. */
-static void
-reconfigure_from_db(struct ovsdb_jsonrpc_server *jsonrpc,
+/* Reconfigures ovsdb-server's remotes based on information in the database. */
+static char *
+reconfigure_remotes(struct ovsdb_jsonrpc_server *jsonrpc,
                     const struct shash *all_dbs, struct sset *remotes)
 {
+    struct ds errors = DS_EMPTY_INITIALIZER;
     struct shash resolved_remotes;
     const char *name;
 
@@ -862,7 +902,7 @@ reconfigure_from_db(struct ovsdb_jsonrpc_server *jsonrpc,
     shash_init(&resolved_remotes);
     SSET_FOR_EACH (name, remotes) {
         if (!strncmp(name, "db:", 3)) {
-            query_db_remotes(name, all_dbs, &resolved_remotes);
+            query_db_remotes(name, all_dbs, &resolved_remotes, &errors);
         } else {
             add_remote(&resolved_remotes, name);
         }
@@ -870,11 +910,42 @@ reconfigure_from_db(struct ovsdb_jsonrpc_server *jsonrpc,
     ovsdb_jsonrpc_server_set_remotes(jsonrpc, &resolved_remotes);
     shash_destroy_free_data(&resolved_remotes);
 
-    /* Configure SSL. */
-    stream_ssl_set_key_and_cert(query_db_string(all_dbs, private_key_file),
-                                query_db_string(all_dbs, certificate_file));
-    stream_ssl_set_ca_cert_file(query_db_string(all_dbs, ca_cert_file),
-                                bootstrap_ca_cert);
+    return errors.string;
+}
+
+static char *
+reconfigure_ssl(const struct shash *all_dbs)
+{
+    struct ds errors = DS_EMPTY_INITIALIZER;
+    const char *resolved_private_key;
+    const char *resolved_certificate;
+    const char *resolved_ca_cert;
+
+    resolved_private_key = query_db_string(all_dbs, private_key_file, &errors);
+    resolved_certificate = query_db_string(all_dbs, certificate_file, &errors);
+    resolved_ca_cert = query_db_string(all_dbs, ca_cert_file, &errors);
+
+    stream_ssl_set_key_and_cert(resolved_private_key, resolved_certificate);
+    stream_ssl_set_ca_cert_file(resolved_ca_cert, bootstrap_ca_cert);
+
+    return errors.string;
+}
+
+static void
+report_error_if_changed(char *error, char **last_errorp)
+{
+    if (error) {
+        if (!*last_errorp || strcmp(error, *last_errorp)) {
+            VLOG_WARN("%s", error);
+            free(*last_errorp);
+            *last_errorp = error;
+            return;
+        }
+        free(error);
+    } else {
+        free(*last_errorp);
+        *last_errorp = NULL;
+    }
 }
 
 static void
@@ -946,9 +1017,9 @@ ovsdb_server_reconnect(struct unixctl_conn *conn, int argc OVS_UNUSED,
  * ovsdb-server services. */
 static void
 ovsdb_server_add_remote(struct unixctl_conn *conn, int argc OVS_UNUSED,
-                        const char *argv[], void *aux_)
+                        const char *argv[], void *config_)
 {
-    struct add_remote_aux *aux = aux_;
+    struct server_config *config = config_;
     const char *remote = argv[1];
 
     const struct ovsdb_column *column;
@@ -958,11 +1029,11 @@ ovsdb_server_add_remote(struct unixctl_conn *conn, int argc OVS_UNUSED,
 
     retval = (strncmp("db:", remote, 3)
               ? NULL
-              : parse_db_column(aux->all_dbs, remote,
+              : parse_db_column(config->all_dbs, remote,
                                 &db, &table, &column));
     if (!retval) {
-        if (sset_add(aux->remotes, remote)) {
-            save_config(aux->config_tmpfile, aux->remotes);
+        if (sset_add(config->remotes, remote)) {
+            save_config(config);
         }
         unixctl_command_reply(conn, NULL);
     } else {
@@ -975,15 +1046,15 @@ ovsdb_server_add_remote(struct unixctl_conn *conn, int argc OVS_UNUSED,
  * that ovsdb-server services. */
 static void
 ovsdb_server_remove_remote(struct unixctl_conn *conn, int argc OVS_UNUSED,
-                           const char *argv[], void *aux_)
+                           const char *argv[], void *config_)
 {
-    struct remove_remote_aux *aux = aux_;
+    struct server_config *config = config_;
     struct sset_node *node;
 
-    node = sset_find(aux->remotes, argv[1]);
+    node = sset_find(config->remotes, argv[1]);
     if (node) {
-        sset_delete(aux->remotes, node);
-        save_config(aux->config_tmpfile, aux->remotes);
+        sset_delete(config->remotes, node);
+        save_config(config);
         unixctl_command_reply(conn, NULL);
     } else {
         unixctl_command_reply_error(conn, "no such remote");
@@ -1011,6 +1082,76 @@ ovsdb_server_list_remotes(struct unixctl_conn *conn, int argc OVS_UNUSED,
     ds_destroy(&s);
 }
 
+
+/* "ovsdb-server/add-db DB": adds the DB to ovsdb-server. */
+static void
+ovsdb_server_add_database(struct unixctl_conn *conn, int argc OVS_UNUSED,
+                          const char *argv[], void *config_)
+{
+    struct server_config *config = config_;
+    const char *filename = argv[1];
+    char *error;
+
+    error = open_db(config, filename);
+    if (!error) {
+        save_config(config);
+        unixctl_command_reply(conn, NULL);
+    } else {
+        unixctl_command_reply_error(conn, error);
+        free(error);
+    }
+}
+
+static void
+ovsdb_server_remove_database(struct unixctl_conn *conn, int argc OVS_UNUSED,
+                             const char *argv[], void *config_)
+{
+    struct server_config *config = config_;
+    struct shash_node *node;
+    struct db *db;
+    bool ok;
+
+    node = shash_find(config->all_dbs, argv[1]);
+    if (!node)  {
+        unixctl_command_reply_error(conn, "Failed to find the database.");
+        return;
+    }
+    db = node->data;
+
+    ok = ovsdb_jsonrpc_server_remove_db(config->jsonrpc, db->db);
+    ovs_assert(ok);
+
+    ovsdb_destroy(db->db);
+    shash_delete(config->all_dbs, node);
+    free(db->filename);
+    free(db);
+
+    save_config(config);
+    unixctl_command_reply(conn, NULL);
+}
+
+static void
+ovsdb_server_list_databases(struct unixctl_conn *conn, int argc OVS_UNUSED,
+                            const char *argv[] OVS_UNUSED, void *all_dbs_)
+{
+    struct shash *all_dbs = all_dbs_;
+    const struct shash_node **nodes;
+    struct ds s;
+    size_t i;
+
+    ds_init(&s);
+
+    nodes = shash_sort(all_dbs);
+    for (i = 0; i < shash_count(all_dbs); i++) {
+        struct db *db = nodes[i]->data;
+        ds_put_format(&s, "%s\n", db->db->schema->name);
+    }
+    free(nodes);
+
+    unixctl_command_reply(conn, ds_cstr(&s));
+    ds_destroy(&s);
+}
+
 static void
 parse_options(int *argcp, char **argvp[],
               struct sset *remotes, char **unixctl_pathp, char **run_command)
@@ -1131,25 +1272,37 @@ usage(void)
     exit(EXIT_SUCCESS);
 }
 
-/* Truncates and replaces the contents of 'config_file' by a representation
- * of 'remotes'. */
+static struct json *
+sset_to_json(const struct sset *sset)
+{
+    struct json *array;
+    const char *s;
+
+    array = json_array_create_empty();
+    SSET_FOR_EACH (s, sset) {
+        json_array_add(array, json_string_create(s));
+    }
+    return array;
+}
+
+/* Truncates and replaces the contents of 'config_file' by a representation of
+ * 'remotes' and 'db_filenames'. */
 static void
-save_config(FILE *config_file, const struct sset *remotes)
+save_config__(FILE *config_file, const struct sset *remotes,
+              const struct sset *db_filenames)
 {
-    const char *remote;
-    struct json *json;
+    struct json *obj;
     char *s;
 
     if (ftruncate(fileno(config_file), 0) == -1) {
         VLOG_FATAL("failed to truncate temporary file (%s)", strerror(errno));
     }
 
-    json = json_array_create_empty();
-    SSET_FOR_EACH (remote, remotes) {
-        json_array_add(json, json_string_create(remote));
-    }
-    s = json_to_string(json, 0);
-    json_destroy(json);
+    obj = json_object_create();
+    json_object_put(obj, "remotes", sset_to_json(remotes));
+    json_object_put(obj, "db_filenames", sset_to_json(db_filenames));
+    s = json_to_string(obj, 0);
+    json_destroy(obj);
 
     if (fseek(config_file, 0, SEEK_SET) != 0
         || fputs(s, config_file) == EOF
@@ -1159,15 +1312,45 @@ save_config(FILE *config_file, const struct sset *remotes)
     free(s);
 }
 
-/* Clears and replaces 'remotes' by a configuration read from 'config_file',
- * which must have been previously written by save_config(). */
+/* Truncates and replaces the contents of 'config_file' by a representation of
+ * 'config'. */
 static void
-load_config(FILE *config_file, struct sset *remotes)
+save_config(struct server_config *config)
+{
+    struct sset db_filenames;
+    struct shash_node *node;
+
+    sset_init(&db_filenames);
+    SHASH_FOR_EACH (node, config->all_dbs) {
+        struct db *db = node->data;
+        sset_add(&db_filenames, db->filename);
+    }
+
+    save_config__(config->config_tmpfile, config->remotes, &db_filenames);
+
+    sset_destroy(&db_filenames);
+}
+
+static void
+sset_from_json(struct sset *sset, const struct json *array)
 {
-    struct json *json;
     size_t i;
 
-    sset_clear(remotes);
+    sset_clear(sset);
+
+    ovs_assert(array->type == JSON_ARRAY);
+    for (i = 0; i < array->u.array.n; i++) {
+        const struct json *elem = array->u.array.elems[i];
+        sset_add(sset, json_string(elem));
+    }
+}
+
+/* Clears and replaces 'remotes' and 'dbnames' by a configuration read from
+ * 'config_file', which must have been previously written by save_config(). */
+static void
+load_config(FILE *config_file, struct sset *remotes, struct sset *db_filenames)
+{
+    struct json *json;
 
     if (fseek(config_file, 0, SEEK_SET) != 0) {
         VLOG_FATAL("seek failed in temporary file (%s)", strerror(errno));
@@ -1176,10 +1359,10 @@ load_config(FILE *config_file, struct sset *remotes)
     if (json->type == JSON_STRING) {
         VLOG_FATAL("reading json failed (%s)", json_string(json));
     }
-    ovs_assert(json->type == JSON_ARRAY);
-    for (i = 0; i < json->u.array.n; i++) {
-        const struct json *remote = json->u.array.elems[i];
-        sset_add(remotes, json_string(remote));
-    }
+    ovs_assert(json->type == JSON_OBJECT);
+
+    sset_from_json(remotes, shash_find_data(json_object(json), "remotes"));
+    sset_from_json(db_filenames,
+                   shash_find_data(json_object(json), "db_filenames"));
     json_destroy(json);
 }
diff --git a/ovsdb/server.c b/ovsdb/server.c
index bf4ef3c..82f55cb 100644
--- a/ovsdb/server.c
+++ b/ovsdb/server.c
@@ -132,6 +132,19 @@ ovsdb_server_add_db(struct ovsdb_server *server, struct ovsdb *db)
     return shash_add_once(&server->dbs, db->schema->name, db);
 }
 
+/* Removes 'db' from the set of databases served out by 'server'.  Returns
+ * true if successful, false if there is no db associated with
+ * db->schema->name. */
+bool
+ovsdb_server_remove_db(struct ovsdb_server *server, struct ovsdb *db)
+{
+    void *data = shash_find_and_delete(&server->dbs, db->schema->name);
+    if (data) {
+        return true;
+    }
+    return false;
+}
+
 /* Destroys 'server'. */
 void
 ovsdb_server_destroy(struct ovsdb_server *server)
diff --git a/ovsdb/server.h b/ovsdb/server.h
index 561f01e..047cbb7 100644
--- a/ovsdb/server.h
+++ b/ovsdb/server.h
@@ -83,6 +83,7 @@ struct ovsdb_server {
 
 void ovsdb_server_init(struct ovsdb_server *);
 bool ovsdb_server_add_db(struct ovsdb_server *, struct ovsdb *);
+bool ovsdb_server_remove_db(struct ovsdb_server *, struct ovsdb *);
 void ovsdb_server_destroy(struct ovsdb_server *);
 
 struct ovsdb_lock_waiter *ovsdb_server_lock(struct ovsdb_server *,
diff --git a/tests/ovsdb-server.at b/tests/ovsdb-server.at
index 2368cbc..16a1d95 100644
--- a/tests/ovsdb-server.at
+++ b/tests/ovsdb-server.at
@@ -164,6 +164,127 @@ AT_CHECK(
 OVSDB_SERVER_SHUTDOWN
 AT_CLEANUP
 
+AT_SETUP([ovsdb-server/add-db and remove-db])
+AT_KEYWORDS([ovsdb server positive])
+ON_EXIT([kill `cat ovsdb-server.pid`])
+OVS_RUNDIR=`pwd`; export OVS_RUNDIR
+OVS_LOGDIR=`pwd`; export OVS_LOGDIR
+ordinal_schema > schema1
+constraint_schema > schema2
+AT_CHECK([ovsdb-tool create db1 schema1], [0], [ignore], [ignore])
+AT_CHECK([ovsdb-tool create db2 schema2], [0], [ignore], [ignore])
+
+# Start ovsdb-server with just a single database - db1.
+AT_CHECK([ovsdb-server --detach --no-chdir --pidfile --remote=punix:socket db1], [0])
+AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/list-dbs],
+  [0], [ordinals
+])
+
+# Add the second database.
+AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/add-db db2], [0])
+AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/list-dbs],
+  [0], [constraints
+ordinals
+])
+
+# The databases are responsive.
+AT_CHECK([ovsdb-client list-tables unix:socket constraints], [0], [ignore], [ignore])
+AT_CHECK([ovsdb-client list-tables unix:socket ordinals], [0], [ignore], [ignore])
+
+# Add an already added database.
+AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/add-db db2], 2, [], [stderr])
+AT_CHECK([sed 's/(.*)/(...)/' stderr], [0],
+  [I/O error: db2: failed to lock lockfile (...)
+ovs-appctl: ovsdb-server: server returned an error
+])
+
+# Add a non-existing database.
+AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/add-db db3], 2, [], [stderr])
+AT_CHECK([sed 's/(.*)/(...)/' stderr], [0],
+  [I/O error: open: db3 failed (...)
+ovs-appctl: ovsdb-server: server returned an error
+])
+
+# Add a remote through a db path in db1.
+AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/add-remote db:ordinals,ordinals,name], [0])
+AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/list-remotes],
+  [0], [db:ordinals,ordinals,name
+punix:socket
+])
+
+# Removing db1 has no effect on its remote.
+AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/remove-db ordinals], [0])
+AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/list-dbs],
+  [0], [constraints
+])
+AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/list-remotes],
+  [0], [db:ordinals,ordinals,name
+punix:socket
+])
+AT_CHECK([ovsdb-client list-tables unix:socket ordinals], [1], [ignore], [ignore])
+
+# Remove db2.
+AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/remove-db constraints], [0])
+AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/list-dbs],
+  [0], [])
+AT_CHECK([ovsdb-client list-tables unix:socket constraints], [1], [ignore], [ignore])
+
+# Remove a non-existent database.
+AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/remove-db ordinals], [2],
+  [], [Failed to find the database.
+ovs-appctl: ovsdb-server: server returned an error
+])
+AT_CLEANUP
+
+AT_SETUP([ovsdb-server/add-db and remove-db with --monitor])
+AT_KEYWORDS([ovsdb server positive])
+# Start ovsdb-server, initially with one db.
+OVS_RUNDIR=`pwd`; export OVS_RUNDIR
+OVS_LOGDIR=`pwd`; export OVS_LOGDIR
+ordinal_schema > schema
+AT_CHECK([ovsdb-tool create db1 schema], [0], [ignore], [ignore])
+ON_EXIT([kill `cat *.pid`])
+AT_CHECK([ovsdb-server -v -vvlog:off --monitor --detach --no-chdir --pidfile --log-file db1])
+
+# Add the second database.
+constraint_schema > schema2
+AT_CHECK([ovsdb-tool create db2 schema2], [0], [ignore], [ignore])
+AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/add-db db2], [0])
+AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/list-dbs],
+  [0], [constraints
+ordinals
+])
+
+# Kill the daemon process, making it look like a segfault,
+# and wait for a new daemon process to get spawned.
+cp ovsdb-server.pid old.pid
+AT_CHECK([kill -SEGV `cat ovsdb-server.pid`])
+OVS_WAIT_WHILE([kill -0 `cat old.pid`])
+OVS_WAIT_UNTIL(
+  [test -s ovsdb-server.pid && test `cat ovsdb-server.pid` != `cat old.pid`])
+AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/list-dbs],
+  [0], [constraints
+ordinals
+])
+
+# Remove the recently added database.
+AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/remove-db constraints])
+AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/list-dbs],
+  [0], [ordinals
+])
+
+# Kill the daemon process, making it look like a segfault,
+# and wait for a new daemon process to get spawned.
+cp ovsdb-server.pid old.pid
+AT_CHECK([kill -SEGV `cat ovsdb-server.pid`])
+OVS_WAIT_WHILE([kill -0 `cat old.pid`])
+OVS_WAIT_UNTIL(
+  [test -s ovsdb-server.pid && test `cat ovsdb-server.pid` != `cat old.pid`])
+AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/list-dbs],
+  [0], [ordinals
+])
+AT_CLEANUP
+
 AT_SETUP([--remote=db: implementation])
 AT_KEYWORDS([ovsdb server positive])
 OVS_RUNDIR=`pwd`; export OVS_RUNDIR
-- 
1.7.2.5




More information about the dev mailing list