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

Gurucharan Shetty shettyg at nicira.com
Wed Jun 26 18:29:36 UTC 2013


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.
--- a/ovsdb/ovsdb-server.c
+++ b/ovsdb/ovsdb-server.c
@@ -1114,7 +1114,7 @@ ovsdb_server_remove_database(struct unixctl_conn
*conn, int argc OVS_UNUSED,
 {
     struct server_config *config = config_;
     struct shash_node *node;
-    const struct db *db;
+    struct db *db;
     bool ok;

     node = shash_find(config->all_dbs, argv[1]);
@@ -1126,7 +1126,11 @@ ovsdb_server_remove_database(struct unixctl_conn
*conn, int argc OVS_UNUSED,

     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);



>
>
>
>> ---
>>  ovsdb/jsonrpc-server.c  |    9 +
>>  ovsdb/jsonrpc-server.h  |    2 +
>>  ovsdb/ovsdb-server.1.in |   23 +++
>>  ovsdb/ovsdb-server.c    |  381
>> ++++++++++++++++++++++++++++++++++-------------
>>  ovsdb/server.c          |   13 ++
>>  ovsdb/server.h          |    1 +
>>  tests/ovsdb-server.at   |  121 +++++++++++++++
>>  7 files changed, 449 insertions(+), 101 deletions(-)
>>
>> diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
>> index 9f99d64..016dd33 100644
>> --- a/ovsdb/jsonrpc-server.c
>> +++ b/ovsdb/jsonrpc-server.c
>> @@ -132,6 +132,15 @@ 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)
>> +{
>> +    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 c11bbb8..e0983ba 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,18 +121,19 @@ 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 db *db;
>>      struct shash all_dbs;
>>      struct shash_node *node;
>> +    char *remotes_error, *ssl_error;
>> +    char *error;
>>      int i;
>>
>>      proctitle_init(argc, argv);
>> @@ -148,29 +153,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++) {
>> -            db = xzalloc(sizeof *db);
>> -            db->filename = argv[i];
>> -            open_db(jsonrpc, db, &all_dbs);
>> -         }
>> -    } else {
>> -        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) {
>> @@ -208,21 +230,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()) {
>> @@ -243,7 +267,10 @@ main(int argc, char *argv[])
>>           * the set of remotes that reconfigure_from_db() 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) {
>> @@ -298,24 +325,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 *
>> @@ -428,8 +462,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;
>> @@ -443,7 +478,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) {
>> @@ -668,7 +704,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;
>> @@ -678,7 +714,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
>> @@ -793,7 +831,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
>> @@ -852,11 +891,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;
>>
>> @@ -864,7 +904,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);
>>          }
>> @@ -872,11 +912,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
>> @@ -951,9 +1022,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;
>> @@ -963,11 +1034,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 {
>> @@ -980,15 +1051,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");
>> @@ -1016,6 +1087,72 @@ 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;
>> +    const 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);
>> +    shash_delete(config->all_dbs, node);
>> +
>> +    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)
>> @@ -1136,25 +1273,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
>> @@ -1164,15 +1313,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));
>> @@ -1181,10 +1360,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
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20130626/99bbf0c7/attachment-0003.html>


More information about the dev mailing list