[ovs-dev] [Replication SM 2/7] ovsdb: Add blacklist_tables

Andy Zhou azhou at ovn.org
Thu Aug 25 00:07:22 UTC 2016


Currently, 'sync-exclude-tables' command line options are simply stored
in a string. Change the implementation to store it in an shash instead
to improve modularity.

One additional benefit of this change is that errors can be detected
and reported to user earlier.  Adde a 'dryrun' option to
set_blacklist_tables() API to make this feature available to the
command line option parsing and unixctl command parsing.

Signed-off-by: Andy Zhou <azhou at ovn.org>
---
 ovsdb/ovsdb-server.c  |  47 ++++++++--------
 ovsdb/replication.c   | 153 +++++++++++++++++++++++++++++++++++++++++---------
 ovsdb/replication.h   |  13 +++--
 tests/ovsdb-server.at |   3 +-
 4 files changed, 159 insertions(+), 57 deletions(-)

diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
index e08c341..b1d7fc5 100644
--- a/ovsdb/ovsdb-server.c
+++ b/ovsdb/ovsdb-server.c
@@ -240,6 +240,7 @@ main(int argc, char *argv[])
     service_start(&argc, &argv);
     fatal_ignore_sigpipe();
     process_init();
+    replication_init();
 
     parse_options(&argc, &argv, &remotes, &unixctl_path, &run_command);
     daemon_become_new_user(false);
@@ -385,7 +386,7 @@ main(int argc, char *argv[])
     sset_destroy(&remotes);
     sset_destroy(&db_filenames);
     unixctl_server_destroy(unixctl);
-    destroy_active_server();
+    replication_destroy();
 
     if (run_process && process_exited(run_process)) {
         int status = process_status(run_process);
@@ -1069,13 +1070,7 @@ ovsdb_server_get_active_ovsdb_server(struct unixctl_conn *conn,
                                      const char *argv[] OVS_UNUSED,
                                      void *arg_ OVS_UNUSED)
 {
-    struct ds s;
-    ds_init(&s);
-
-    ds_put_format(&s, "%s\n", get_active_ovsdb_server());
-
-    unixctl_command_reply(conn, ds_cstr(&s));
-    ds_destroy(&s);
+    unixctl_command_reply(conn, get_active_ovsdb_server());
 }
 
 static void
@@ -1084,10 +1079,10 @@ ovsdb_server_connect_active_ovsdb_server(struct unixctl_conn *conn,
                                          const char *argv[] OVS_UNUSED,
                                          void *arg_ OVS_UNUSED)
 {
-    if (!is_backup_server) {
+    if (is_backup_server) {
         replication_init();
-        is_backup_server = true;
     }
+    is_backup_server = true;
     unixctl_command_reply(conn, NULL);
 }
 
@@ -1109,8 +1104,15 @@ ovsdb_server_set_sync_excluded_tables(struct unixctl_conn *conn,
                                       const char *argv[],
                                       void *arg_ OVS_UNUSED)
 {
-    set_tables_blacklist(argv[1]);
-    unixctl_command_reply(conn, NULL);
+    char *err = set_blacklist_tables(argv[1], true);
+
+    if (!err) {
+        if (is_backup_server) {
+            replication_init();
+        }
+        set_blacklist_tables(argv[1], false);
+    }
+    unixctl_command_reply(conn, err);
 }
 
 static void
@@ -1119,17 +1121,7 @@ ovsdb_server_get_sync_excluded_tables(struct unixctl_conn *conn,
                                  const char *argv[] OVS_UNUSED,
                                  void *arg_ OVS_UNUSED)
 {
-    struct ds s;
-    const char *table_name;
-    struct sset table_blacklist = get_tables_blacklist();
-
-    ds_init(&s);
-
-    SSET_FOR_EACH(table_name, &table_blacklist) {
-        ds_put_format(&s, "%s\n", table_name);
-    }
-
-    unixctl_command_reply(conn, ds_cstr(&s));
+    unixctl_command_reply(conn, get_blacklist_tables());
 }
 
 static void
@@ -1468,9 +1460,14 @@ parse_options(int *argcp, char **argvp[],
             is_backup_server = true;
             break;
 
-        case OPT_SYNC_EXCLUDE:
-            set_tables_blacklist(optarg);
+        case OPT_SYNC_EXCLUDE: {
+            char *err = set_blacklist_tables(optarg, false);
+            if (err) {
+                printf("%s\n", err);
+                exit(EXIT_FAILURE);
+            }
             break;
+        }
 
         case '?':
             exit(EXIT_FAILURE);
diff --git a/ovsdb/replication.c b/ovsdb/replication.c
index 6681a20..ebd348c 100644
--- a/ovsdb/replication.c
+++ b/ovsdb/replication.c
@@ -20,6 +20,7 @@
 #include "replication.h"
 
 #include "condition.h"
+#include "openvswitch/dynamic-string.h"
 #include "openvswitch/json.h"
 #include "openvswitch/vlog.h"
 #include "jsonrpc.h"
@@ -38,7 +39,6 @@ VLOG_DEFINE_THIS_MODULE(replication)
 static char *active_ovsdb_server;
 static struct jsonrpc *rpc;
 static struct sset monitored_tables = SSET_INITIALIZER(&monitored_tables);
-static struct sset tables_blacklist = SSET_INITIALIZER(&tables_blacklist);
 static bool reset_dbs = true;
 
 static struct jsonrpc *open_jsonrpc(const char *server);
@@ -76,11 +76,20 @@ static struct ovsdb_error *execute_update(struct ovsdb_txn *txn,
                                           struct ovsdb_table *table,
                                           struct json *new);
 
+/* Maps from db name to sset of table names. */
+static struct shash blacklist_tables = SHASH_INITIALIZER(&blacklist_tables);
+
+static void blacklist_tables_clear(void);
+static void blacklist_tables_add(const char *database, const char *table);
+static bool blacklist_tables_find(const char *database, const char* table);
+
+
 void
 replication_init(void)
 {
-    sset_init(&monitored_tables);
-    sset_init(&tables_blacklist);
+    if (rpc) {
+        disconnect_active_server();
+    }
     reset_dbs = true;
 }
 
@@ -136,19 +145,118 @@ get_active_ovsdb_server(void)
     return active_ovsdb_server;
 }
 
-void
-set_tables_blacklist(const char *blacklist)
+/* Parse 'blacklist' to rebuild 'blacklist_tables'. The current
+ * black list tables will be wiped out, regardless if 'blacklist'
+ * can be parsed or not.
+ *
+ * On error, Returns the error string which the caller is
+ * responsible for freeing. Return NULL otherwise.    */
+char *
+set_blacklist_tables(const char *blacklist, bool dryrun)
 {
-    replication_init();
+    struct sset set = SSET_INITIALIZER(&set);
+    char *err = NULL;
+
     if (blacklist) {
-        sset_from_delimited_string(&tables_blacklist, blacklist, ",");
+        const char *longname;
+
+        if (!dryrun) {
+            blacklist_tables_clear();
+        }
+
+        sset_from_delimited_string(&set, blacklist, " ,");
+        SSET_FOR_EACH (longname, &set) {
+            char *database = xstrdup(longname), *table = NULL;
+            strtok_r(database, ":", &table);
+            if (table && !dryrun) {
+                blacklist_tables_add(database, table);
+            }
+
+            free(database);
+            if (!table) {
+                err = xasprintf("Can't parse black list table: %s", longname);
+                goto done;
+            }
+        }
+    }
+
+done:
+    sset_destroy(&set);
+    if (err && !dryrun) {
+        /* On error, destory the partially built 'blacklist_tables'. */
+        blacklist_tables_clear();
+    }
+    return err;
+}
+
+char *
+get_blacklist_tables(void)
+{
+    struct shash_node *node;
+    struct sset set = SSET_INITIALIZER(&set);
+
+    SHASH_FOR_EACH (node, &blacklist_tables) {
+        const char *database = node->name;
+        const char *table;
+        struct sset *tables = node->data;
+
+        SSET_FOR_EACH (table, tables) {
+            struct ds ds= DS_EMPTY_INITIALIZER;
+            ds_put_format(&ds, "%s:%s", database, table);
+            sset_add(&set, ds_steal_cstr(&ds));
+        }
+    }
+
+    /* Output the table list in an sorted order, so that
+     * the output string will not depend on the hash function
+     * that used to implement the hmap data structure. This is
+     * only useful for writting unit tests.  */
+    const char **sorted = sset_sort(&set);
+    struct ds ds= DS_EMPTY_INITIALIZER;
+    size_t i;
+    for (i = 0; i < sset_count(&set); i++) {
+        ds_put_format(&ds, "%s,", sorted[i]);
+    }
+
+    ds_chomp(&ds, ',');
+
+    free(sorted);
+    sset_destroy(&set);
+
+    return ds_steal_cstr(&ds);
+}
+
+static void
+blacklist_tables_clear(void)
+{
+    struct shash_node *node;
+    SHASH_FOR_EACH (node, &blacklist_tables) {
+        struct sset *tables = node->data;
+        sset_destroy(tables);
+    }
+
+    shash_clear_free_data(&blacklist_tables);
+}
+
+static void
+blacklist_tables_add(const char *database, const char *table)
+{
+    struct sset *tables = shash_find_data(&blacklist_tables, database);
+
+    if (!tables) {
+        tables = xmalloc(sizeof *tables);
+        sset_init(tables);
+        shash_add(&blacklist_tables, database, tables);
     }
+
+    sset_add(tables, table);
 }
 
-struct sset
-get_tables_blacklist(void)
+static bool
+blacklist_tables_find(const char *database, const char *table)
 {
-    return tables_blacklist;
+    struct sset *tables = shash_find_data(&blacklist_tables, database);
+    return tables ? sset_contains(tables, table) : false;
 }
 
 void
@@ -157,15 +265,15 @@ disconnect_active_server(void)
     jsonrpc_close(rpc);
     rpc = NULL;
     sset_clear(&monitored_tables);
-    sset_clear(&tables_blacklist);
 }
 
 void
-destroy_active_server(void)
+replication_destroy(void)
 {
     disconnect_active_server();
     sset_destroy(&monitored_tables);
-    sset_destroy(&tables_blacklist);
+    blacklist_tables_clear();
+    shash_destroy(&blacklist_tables);
 
     if (active_ovsdb_server) {
         free(active_ovsdb_server);
@@ -210,18 +318,14 @@ reset_database(struct ovsdb *db, struct ovsdb_txn *txn)
     struct shash_node *table_node;
 
     SHASH_FOR_EACH (table_node, &db->tables) {
-        struct ovsdb_table *table = table_node->data;
-        struct ovsdb_row *row;
-
-        /* Do not reset if table is blacklisted. */
-        char *blacklist_item = xasprintf(
-            "%s%s%s", db->schema->name, ":", table_node->name);
-        if (!sset_contains(&tables_blacklist, blacklist_item)) {
+        /* Delete all rows if the table is not blacklisted. */
+        if (!blacklist_tables_find(db->schema->name, table_node->name)) {
+            struct ovsdb_table *table = table_node->data;
+            struct ovsdb_row *row;
             HMAP_FOR_EACH (row, hmap_node, &table->rows) {
                 ovsdb_txn_row_delete(txn, row);
             }
         }
-        free(blacklist_item);
     }
 }
 
@@ -352,13 +456,10 @@ send_monitor_requests(struct shash *all_dbs)
                 for (int j = 0; j < n; j++) {
                     struct ovsdb_table_schema *table = nodes[j]->data;
 
-                    /* Check if table is not blacklisted. */
-                    char *blacklist_item = xasprintf(
-                        "%s%s%s", db_name, ":", table->name);
-                    if (!sset_contains(&tables_blacklist, blacklist_item)) {
+                    /* Monitor all tables not blacklisted. */
+                    if (!blacklist_tables_find(db_name, table->name)) {
                         add_monitored_table(table, monitor_request);
                     }
-                    free(blacklist_item);
                 }
                 free(nodes);
 
diff --git a/ovsdb/replication.h b/ovsdb/replication.h
index d80afb2..1de9c8b 100644
--- a/ovsdb/replication.h
+++ b/ovsdb/replication.h
@@ -18,8 +18,10 @@
 #ifndef REPLICATION_H
 #define REPLICATION_H 1
 
+#include <stdbool.h>
 #include "openvswitch/shash.h"
 
+
 struct db {
     /* Initialized in main(). */
     char *filename;
@@ -30,16 +32,19 @@ struct db {
     struct ovsdb_txn *txn;
 };
 
+/* Functions to be called from the main loop. */
 void replication_init(void);
 void replication_run(struct shash *dbs);
 void replication_wait(void);
+void replication_destroy(void);
+void replication_usage(void);
+
+/* Unixctl APIs */
 void set_active_ovsdb_server(const char *remote_server);
 const char *get_active_ovsdb_server(void);
-void set_tables_blacklist(const char *blacklist);
-struct sset get_tables_blacklist(void);
+char *set_blacklist_tables(const char *blacklist, bool dryrun);
+char *get_blacklist_tables(void);
 void disconnect_active_server(void);
-void destroy_active_server(void);
 const struct db *find_db(const struct shash *all_dbs, const char *db_name);
-void replication_usage(void);
 
 #endif /* ovsdb/replication.h */
diff --git a/tests/ovsdb-server.at b/tests/ovsdb-server.at
index 0436de8..c1e7d96 100644
--- a/tests/ovsdb-server.at
+++ b/tests/ovsdb-server.at
@@ -1114,8 +1114,7 @@ on_exit 'kill `cat *.pid`'
 AT_CHECK([ovsdb-server --detach --no-chdir --pidfile --sync-exclude-tables=mydb:db1,mydb:db2 db])
 
 AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/get-sync-excluded-tables],
-  [0], [mydb:db2
-mydb:db1
+  [0], [mydb:db1,mydb:db2
 ])
 AT_CLEANUP
 
-- 
1.9.1




More information about the dev mailing list