Make ovs-vswitchd report when it is done configuring; make ovs-vsctl wait.

Until now the ovsdb-based vswitch has provided no way to know when it has
finished applying the configuration from the database.  This commit
introduces a way:

  * The client who wants to wait increments the "next_cfg" column of the
    Open_vSwitch record.

  * When ovs-vswitchd finishes reconfiguring, it sets the value of the
    "cur_cfg" column to that of the "next_cfg" column.

  * The client waits until the "cur_cfg" column is at least as great as
    the value it set into "next_cfg".

This allows us to drop the 5-second sleep in interface-reconfigure.

diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index f72e18762f0857f6e89ed74dd4869c87c1252918..877fa3e284bfcb1cfea25b29059ff51db817ec77 100644 (file)
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -19,6 +19,7 @@
 
 #include <assert.h>
 #include <errno.h>
+#include <inttypes.h>
 #include <limits.h>
 #include <stdlib.h>
 
@@ -82,6 +83,13 @@ struct ovsdb_idl_txn {
     enum ovsdb_idl_txn_status status;
     bool dry_run;
     struct ds comment;
+
+    /* Increments. */
+    char *inc_table;
+    char *inc_column;
+    struct json *inc_where;
+    unsigned int inc_index;
+    int64_t inc_new_value;
 };
 
 static struct vlog_rate_limit syntax_rl = VLOG_RATE_LIMIT_INIT(1, 5);
@@ -770,6 +778,8 @@ const char *
 ovsdb_idl_txn_status_to_string(enum ovsdb_idl_txn_status status)
 {
     switch (status) {
+    case TXN_UNCHANGED:
+        return "unchanged";
     case TXN_INCOMPLETE:
         return "incomplete";
     case TXN_ABORTED:
@@ -796,6 +806,9 @@ ovsdb_idl_txn_create(struct ovsdb_idl *idl)
     hmap_init(&txn->txn_rows);
     txn->dry_run = false;
     ds_init(&txn->comment);
+    txn->inc_table = NULL;
+    txn->inc_column = NULL;
+    txn->inc_where = NULL;
     return txn;
 }
 
@@ -814,6 +827,16 @@ ovsdb_idl_txn_set_dry_run(struct ovsdb_idl_txn *txn)
     txn->dry_run = true;
 }
 
+void
+ovsdb_idl_txn_increment(struct ovsdb_idl_txn *txn, const char *table,
+                        const char *column, const struct json *where)
+{
+    assert(!txn->inc_table);
+    txn->inc_table = xstrdup(table);
+    txn->inc_column = xstrdup(column);
+    txn->inc_where = where ? json_clone(where) : json_array_create_empty();
+}
+
 void
 ovsdb_idl_txn_destroy(struct ovsdb_idl_txn *txn)
 {
@@ -822,6 +845,9 @@ ovsdb_idl_txn_destroy(struct ovsdb_idl_txn *txn)
     }
     ovsdb_idl_txn_abort(txn);
     ds_destroy(&txn->comment);
+    free(txn->inc_table);
+    free(txn->inc_column);
+    json_destroy(txn->inc_where);
     free(txn);
 }
 
@@ -1052,6 +1078,36 @@ ovsdb_idl_txn_commit(struct ovsdb_idl_txn *txn)
         }
     }
 
+    /* Add increment. */
+    if (txn->inc_table && any_updates) {
+        struct json *op;
+
+        txn->inc_index = operations->u.array.n;
+
+        op = json_object_create();
+        json_object_put_string(op, "op", "mutate");
+        json_object_put_string(op, "table", txn->inc_table);
+        json_object_put(op, "where",
+                        substitute_uuids(json_clone(txn->inc_where), txn));
+        json_object_put(op, "mutations",
+                        json_array_create_1(
+                            json_array_create_3(
+                                json_string_create(txn->inc_column),
+                                json_string_create("+="),
+                                json_integer_create(1))));
+        json_array_add(operations, op);
+
+        op = json_object_create();
+        json_object_put_string(op, "op", "select");
+        json_object_put_string(op, "table", txn->inc_table);
+        json_object_put(op, "where",
+                        substitute_uuids(json_clone(txn->inc_where), txn));
+        json_object_put(op, "columns",
+                        json_array_create_1(json_string_create(
+                                                txn->inc_column)));
+        json_array_add(operations, op);
+    }
+
     if (txn->comment.length) {
         struct json *op = json_object_create();
         json_object_put_string(op, "op", "comment");
@@ -1066,7 +1122,7 @@ ovsdb_idl_txn_commit(struct ovsdb_idl_txn *txn)
     }
 
     if (!any_updates) {
-        txn->status = TXN_SUCCESS;
+        txn->status = TXN_UNCHANGED;
     } else if (!jsonrpc_session_send(
                    txn->idl->session,
                    jsonrpc_create_request(
@@ -1082,6 +1138,13 @@ ovsdb_idl_txn_commit(struct ovsdb_idl_txn *txn)
     return txn->status;
 }
 
+int64_t
+ovsdb_idl_txn_get_increment_new_value(const struct ovsdb_idl_txn *txn)
+{
+    assert(txn->status == TXN_SUCCESS);
+    return txn->inc_new_value;
+}
+
 void
 ovsdb_idl_txn_abort(struct ovsdb_idl_txn *txn)
 {
@@ -1206,6 +1269,75 @@ ovsdb_idl_txn_find(struct ovsdb_idl *idl, const struct json *id)
     return NULL;
 }
 
+static bool
+check_json_type(const struct json *json, enum json_type type, const char *name)
+{
+    if (!json) {
+        VLOG_WARN_RL(&syntax_rl, "%s is missing", name);
+        return false;
+    } else if (json->type != type) {
+        VLOG_WARN_RL(&syntax_rl, "%s is %s instead of %s",
+                     name, json_type_to_string(json->type),
+                     json_type_to_string(type));
+        return false;
+    } else {
+        return true;
+    }
+}
+
+static bool
+ovsdb_idl_txn_process_inc_reply(struct ovsdb_idl_txn *txn,
+                                const struct json_array *results)
+{
+    struct json *count, *rows, *row, *column;
+    struct shash *mutate, *select;
+
+    if (txn->inc_index + 2 > results->n) {
+        VLOG_WARN_RL(&syntax_rl, "reply does not contain enough operations "
+                     "for increment (has %u, needs %u)",
+                     results->n, txn->inc_index + 2);
+        return false;
+    }
+
+    /* We know that this is a JSON objects because the loop in
+     * ovsdb_idl_txn_process_reply() checked. */
+    mutate = json_object(results->elems[txn->inc_index]);
+    count = shash_find_data(mutate, "count");
+    if (!check_json_type(count, JSON_INTEGER, "\"mutate\" reply \"count\"")) {
+        return false;
+    }
+    if (count->u.integer != 1) {
+        VLOG_WARN_RL(&syntax_rl,
+                     "\"mutate\" reply \"count\" is %"PRId64" instead of 1",
+                     count->u.integer);
+        return false;
+    }
+
+    select = json_object(results->elems[txn->inc_index + 1]);
+    rows = shash_find_data(select, "rows");
+    if (!check_json_type(rows, JSON_ARRAY, "\"select\" reply \"rows\"")) {
+        return false;
+    }
+    if (rows->u.array.n != 1) {
+        VLOG_WARN_RL(&syntax_rl, "\"select\" reply \"rows\" has %u elements "
+                     "instead of 1",
+                     rows->u.array.n);
+        return false;
+    }
+    row = rows->u.array.elems[0];
+    if (!check_json_type(row, JSON_OBJECT, "\"select\" reply row")) {
+        return false;
+    }
+    column = shash_find_data(json_object(row), txn->inc_column);
+    if (!check_json_type(column, JSON_INTEGER,
+                         "\"select\" reply inc column")) {
+        return false;
+    }
+    txn->inc_new_value = column->u.integer;
+    return true;
+}
+
+
 static bool
 ovsdb_idl_txn_process_reply(struct ovsdb_idl *idl,
                             const struct jsonrpc_msg *msg)
@@ -1259,6 +1391,14 @@ ovsdb_idl_txn_process_reply(struct ovsdb_idl *idl,
             }
         }
 
+        if (txn->inc_table
+            && !soft_errors
+            && !hard_errors
+            && !ovsdb_idl_txn_process_inc_reply(txn,
+                                                json_array(msg->result))) {
+            hard_errors++;
+        }
+
         status = (hard_errors ? TXN_ERROR
                   : soft_errors ? TXN_TRY_AGAIN
                   : TXN_SUCCESS);

diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h
index 0a1fbc6ba46646adb1a884da9e8884d448aaed93..fb551b485ec6b5355e68e53fe4d6dc5481226fcf 100644 (file)
--- a/lib/ovsdb-idl.h
+++ b/lib/ovsdb-idl.h
@@ -16,6 +16,9 @@
 #ifndef OVSDB_IDL_H
 #define OVSDB_IDL_H 1
 
+#include <stdint.h>
+
+struct json;
 struct ovsdb_idl_class;
 
 struct ovsdb_idl *ovsdb_idl_create(const char *remote,
@@ -29,6 +32,7 @@ unsigned int ovsdb_idl_get_seqno(const struct ovsdb_idl *);
 void ovsdb_idl_force_reconnect(struct ovsdb_idl *);
 
 enum ovsdb_idl_txn_status {
+    TXN_UNCHANGED,              /* Transaction didn't include any changes. */
     TXN_INCOMPLETE,             /* Commit in progress, please wait. */
     TXN_ABORTED,                /* ovsdb_idl_txn_abort() called. */
     TXN_SUCCESS,                /* Commit successful. */
@@ -43,9 +47,12 @@ const char *ovsdb_idl_txn_status_to_string(enum ovsdb_idl_txn_status);
 struct ovsdb_idl_txn *ovsdb_idl_txn_create(struct ovsdb_idl *);
 void ovsdb_idl_txn_add_comment(struct ovsdb_idl_txn *, const char *);
 void ovsdb_idl_txn_set_dry_run(struct ovsdb_idl_txn *);
+void ovsdb_idl_txn_increment(struct ovsdb_idl_txn *, const char *table,
+                             const char *column, const struct json *where);
 void ovsdb_idl_txn_destroy(struct ovsdb_idl_txn *);
 void ovsdb_idl_txn_wait(const struct ovsdb_idl_txn *);
 enum ovsdb_idl_txn_status ovsdb_idl_txn_commit(struct ovsdb_idl_txn *);
+int64_t ovsdb_idl_txn_get_increment_new_value(const struct ovsdb_idl_txn *);
 void ovsdb_idl_txn_abort(struct ovsdb_idl_txn *);
 
 #endif /* ovsdb-idl.h */

diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
index cd4cab3d2c60864b602bf037a1c3c5caaa262652..b1e7948498bd21eea1e4718676eb7088ae55102e 100644 (file)
--- a/tests/ovs-vsctl.at
+++ b/tests/ovs-vsctl.at
@@ -23,17 +23,17 @@ dnl RUN_OVS_VSCTL(COMMAND, ...)
 dnl
 dnl Executes each ovs-vsctl COMMAND.
 m4_define([RUN_OVS_VSCTL],
-  [m4_foreach([command], [$@], [ovs-vsctl -vreconnect:ANY:emer --db=unix:socket -- command
+  [m4_foreach([command], [$@], [ovs-vsctl --no-wait -vreconnect:ANY:emer --db=unix:socket -- command
 ])])
 m4_define([RUN_OVS_VSCTL_ONELINE],
-  [m4_foreach([command], [$@], [ovs-vsctl -vreconnect:ANY:emer --db=unix:socket --oneline -- command
+  [m4_foreach([command], [$@], [ovs-vsctl --no-wait -vreconnect:ANY:emer --db=unix:socket --oneline -- command
 ])])
 
 dnl RUN_OVS_VSCTL_TOGETHER(COMMAND, ...)
 dnl
 dnl Executes each ovs-vsctl COMMAND in a single run of ovs-vsctl.
 m4_define([RUN_OVS_VSCTL_TOGETHER],
-  [ovs-vsctl -vreconnect:ANY:emer --db=unix:socket --oneline dnl
+  [ovs-vsctl --no-wait -vreconnect:ANY:emer --db=unix:socket --oneline dnl
 m4_foreach([command], [$@], [ -- command])])
 
 dnl CHECK_BRIDGES([BRIDGE, PARENT, VLAN], ...)

diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
index 65914194bef8bc9ac9712ca1e996634a0006b04f..973ccac88b00f1ae161087b9ef524a7cf01e7836 100644 (file)
--- a/tests/ovsdb-idl.at
+++ b/tests/ovsdb-idl.at
@@ -167,6 +167,17 @@ OVSDB_CHECK_IDL([simple idl, writing via IDL],
 005: done
 ]])
 
+OVSDB_CHECK_IDL([simple idl, increment operation],
+  [['[{"op": "insert",
+       "table": "simple",
+       "row": {}}]']],
+  [['set 0 r 2.0, increment simple i']],
+  [[000: i=0 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1>
+001: commit, status=success, increment=1
+002: i=1 r=2 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1>
+003: done
+]])
+
 OVSDB_CHECK_IDL([self-linking idl, consistent ops],
   [],
   [['[{"op": "insert",

diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c
index a0476a922becb52570a2410b7ab415da7a9ddab4..ede792262e443c8fe5ee108c799e9511065650fa 100644 (file)
--- a/tests/test-ovsdb.c
+++ b/tests/test-ovsdb.c
@@ -1553,6 +1553,7 @@ idl_set(struct ovsdb_idl *idl, char *commands, int step)
     char *cmd, *save_ptr1 = NULL;
     struct ovsdb_idl_txn *txn;
     enum ovsdb_idl_txn_status status;
+    bool increment = false;
 
     txn = ovsdb_idl_txn_create(idl);
     for (cmd = strtok_r(commands, ",", &save_ptr1); cmd;
@@ -1614,6 +1615,12 @@ idl_set(struct ovsdb_idl *idl, char *commands, int step)
                           "i=%d", atoi(arg1));
             }
             idltest_simple_delete(s);
+        } else if (!strcmp(name, "increment")) {
+            if (!arg2 || arg3) {
+                ovs_fatal(0, "\"set\" command requires 2 arguments");
+            }
+            ovsdb_idl_txn_increment(txn, arg1, arg2, NULL);
+            increment = true;
         } else {
             ovs_fatal(0, "unknown command %s", name);
         }
@@ -1625,8 +1632,13 @@ idl_set(struct ovsdb_idl *idl, char *commands, int step)
         ovsdb_idl_txn_wait(txn);
         poll_block();
     }
-    printf("%03d: commit, status=%s\n",
+    printf("%03d: commit, status=%s",
            step, ovsdb_idl_txn_status_to_string(status));
+    if (increment) {
+        printf(", increment=%"PRId64,
+               ovsdb_idl_txn_get_increment_new_value(txn));
+    }
+    putchar('\n');
     ovsdb_idl_txn_destroy(txn);
 }
 

diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index bc6e4dbb11bc3ceb4eff53d488538918d2bff31b..941e6b7c71edce3e5b9584a431bd7a6bb866036a 100644 (file)
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -29,6 +29,7 @@
 #include "compiler.h"
 #include "dirs.h"
 #include "dynamic-string.h"
+#include "json.h"
 #include "ovsdb-idl.h"
 #include "poll-loop.h"
 #include "svec.h"
@@ -48,6 +49,9 @@ static bool oneline;
 /* --dry-run: Do not commit any changes. */
 static bool dry_run;
 
+/* --no-wait: Wait for ovs-vswitchd to reload its configuration? */
+static bool wait_for_reload = true;
+
 /* --timeout: Time to wait for a connection to 'db'. */
 static int timeout = 5;
 
@@ -188,7 +192,7 @@ parse_options(int argc, char *argv[])
             break;
 
         case OPT_NO_WAIT:
-            /* XXX not yet implemented */
+            wait_for_reload = false;
             break;
 
         case OPT_DRY_RUN:
@@ -1208,6 +1212,20 @@ static void run_vsctl_command(int argc, char *argv[],
                               const struct ovsrec_open_vswitch *ovs,
                               struct ds *output);
 
+static struct json *
+where_uuid_equals(const struct uuid *uuid)
+{
+    return
+        json_array_create_1(
+            json_array_create_3(
+                json_string_create("_uuid"),
+                json_string_create("=="),
+                json_array_create_2(
+                    json_string_create("uuid"),
+                    json_string_create_nocopy(
+                        xasprintf(UUID_FMT, UUID_ARGS(uuid))))));
+}
+
 static void
 do_vsctl(int argc, char *argv[], struct ovsdb_idl *idl)
 {
@@ -1215,6 +1233,7 @@ do_vsctl(int argc, char *argv[], struct ovsdb_idl *idl)
     const struct ovsrec_open_vswitch *ovs;
     enum ovsdb_idl_txn_status status;
     struct ds comment, *output;
+    int64_t next_cfg;
     int n_output;
     int i, start;
 
@@ -1237,6 +1256,13 @@ do_vsctl(int argc, char *argv[], struct ovsdb_idl *idl)
         ovs = ovsrec_open_vswitch_insert(txn);
     }
 
+    if (wait_for_reload) {
+        struct json *where = where_uuid_equals(&ovs->header_.uuid);
+        ovsdb_idl_txn_increment(txn, "Open_vSwitch", "next_cfg",
+                                where);
+        json_destroy(where);
+    }
+
     output = xmalloc(argc * sizeof *output);
     n_output = 0;
     for (start = i = 0; i <= argc; i++) {
@@ -1256,6 +1282,9 @@ do_vsctl(int argc, char *argv[], struct ovsdb_idl *idl)
         ovsdb_idl_txn_wait(txn);
         poll_block();
     }
+    if (wait_for_reload && status == TXN_SUCCESS) {
+        next_cfg = ovsdb_idl_txn_get_increment_new_value(txn);
+    }
     ovsdb_idl_txn_destroy(txn);
 
     switch (status) {
@@ -1266,6 +1295,7 @@ do_vsctl(int argc, char *argv[], struct ovsdb_idl *idl)
         /* Should not happen--we never call ovsdb_idl_txn_abort(). */
         vsctl_fatal("transaction aborted");
 
+    case TXN_UNCHANGED:
     case TXN_SUCCESS:
         break;
 
@@ -1308,6 +1338,23 @@ do_vsctl(int argc, char *argv[], struct ovsdb_idl *idl)
             fputs(ds_cstr(ds), stdout);
         }
     }
+
+    if (wait_for_reload && status != TXN_UNCHANGED) {
+        for (;;) {
+            const struct ovsrec_open_vswitch *ovs;
+
+            ovsdb_idl_run(idl);
+            OVSREC_OPEN_VSWITCH_FOR_EACH (ovs, idl) {
+                if (ovs->cur_cfg >= next_cfg) {
+                    goto done;
+                }
+            }
+            ovsdb_idl_wait(idl);
+            poll_block();
+        }
+    done: ;
+    }
+
     exit(EXIT_SUCCESS);
 }
 

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 2b4a3f0cd94e3df10adcbddf54116c97c7764feb..040c7f382de42564ed97b0c170f9333be8e26a66 100644 (file)
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -754,6 +754,8 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
         iterate_and_prune_ifaces(br, set_iface_properties, NULL);
     }
 
+    ovsrec_open_vswitch_set_cur_cfg(ovs_cfg, ovs_cfg->next_cfg);
+
     ovsdb_idl_txn_commit(txn);
     ovsdb_idl_txn_destroy(txn); /* XXX */
 }

diff --git a/vswitchd/vswitch-idl.ovsidl b/vswitchd/vswitch-idl.ovsidl
index 9fd07dc4e951232972aaac115e9d8dd66c951bc5..3fcf385d034fb7d33f5752e7d1581dcbf6823e99 100644 (file)
--- a/vswitchd/vswitch-idl.ovsidl
+++ b/vswitchd/vswitch-idl.ovsidl
@@ -25,7 +25,13 @@
          "type": {"key": "uuid", "keyRefTable": "Controller", "min": 0, "max": 1}},
        "ssl": {
          "comment": "SSL used globally by the daemon.",
-         "type": {"key": "uuid", "keyRefTable": "SSL", "min": 0, "max": 1}}}},
+         "type": {"key": "uuid", "keyRefTable": "SSL", "min": 0, "max": 1}},
+       "next_cfg": {
+         "comment": "Sequence number for client to increment when it modifies the configuration and wishes to wait for Open vSwitch to finish applying the changes.",
+         "type": "integer"},
+       "cur_cfg": {
+         "comment": "Sequence number that Open vSwitch sets to the current value of 'next_cfg' after it finishing applying a set of configuration changes.",
+         "type": "integer"}}},
    "Bridge": {
      "comment": "Configuration for a bridge within an Open_vSwitch.",
      "columns": {

diff --git a/xenserver/opt_xensource_libexec_interface-reconfigure b/xenserver/opt_xensource_libexec_interface-reconfigure
index de6043d1619bd0c0c4364a17efb1f05439b0dd2f..d346cdc4b1b78812c3fccdef670ef000a6a1285a 100755 (executable)
--- a/xenserver/opt_xensource_libexec_interface-reconfigure
+++ b/xenserver/opt_xensource_libexec_interface-reconfigure
@@ -1257,7 +1257,6 @@ def datapath_modify_config(commands):
                      + [c for c in commands if not c.startswith('#')])
     if not rc:
         raise Error("Failed to modify vswitch configuration")
-    run_command(['/bin/sleep', '5']) # XXX
     return True
 
 #