ovsdb-server: Reliably report status of inbound connections.

ovsdb_jsonrpc_server keeps track of its remotes in a shash indexed on the
remote name specified in the database Manager record, but
ovsdb_jsonrpc_server_get_remote_status() added the name returned by
jsonrpc_session_get_name() to the shash returned to the ovsdb-server code.
If that name happened to be different (which is entirely possible because
the latter returns a "canonicalized" name in some cases) then the
ovsdb-server code couldn't find it.  Furthermore, if an inbound (e.g.
"ptcp:") Manager got a connection and then lost it, the status info in
that Manager never got updated to reflect that, because the code considered
that that "couldn't happen" and didn't bother to do any updates.

This commit simplifies the logic.  Now ovsdb-server just asks for a single
status record at a time, using the name that is indexed in the
ovsdb_jsonrpc_server shash, avoiding that whole issue.

diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index e999ada..06e3acc 100644 (file)
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -22,6 +22,7 @@
 
 #include "bitmap.h"
 #include "column.h"
+#include "dynamic-string.h"
 #include "json.h"
 #include "jsonrpc.h"
 #include "ovsdb-error.h"
@@ -53,9 +54,9 @@ static void ovsdb_jsonrpc_session_close_all(struct ovsdb_jsonrpc_remote *);
 static void ovsdb_jsonrpc_session_reconnect_all(struct ovsdb_jsonrpc_remote *);
 static void ovsdb_jsonrpc_session_set_all_options(
     struct ovsdb_jsonrpc_remote *, const struct ovsdb_jsonrpc_options *);
-static void ovsdb_jsonrpc_session_get_status(
+static bool ovsdb_jsonrpc_session_get_status(
     const struct ovsdb_jsonrpc_remote *,
-    struct shash *);
+    struct ovsdb_jsonrpc_remote_status *);
 
 /* Triggers. */
 static void ovsdb_jsonrpc_trigger_create(struct ovsdb_jsonrpc_session *,
@@ -198,19 +199,23 @@ ovsdb_jsonrpc_server_del_remote(struct shash_node *node)
     free(remote);
 }
 
-void
-ovsdb_jsonrpc_server_get_remote_status(const struct ovsdb_jsonrpc_server *svr,
-                                       struct shash *statuses)
+/* Stores status information for the remote named 'target', which should have
+ * been configured on 'svr' with a call to ovsdb_jsonrpc_server_set_remotes(),
+ * into '*status'.  On success returns true, on failure (if 'svr' doesn't have
+ * a remote named 'target' or if that remote is an inbound remote that has no
+ * active connections) returns false.  On failure, 'status' will be zeroed.
+ */
+bool
+ovsdb_jsonrpc_server_get_remote_status(
+    const struct ovsdb_jsonrpc_server *svr, const char *target,
+    struct ovsdb_jsonrpc_remote_status *status)
 {
-    struct shash_node *node;
+    const struct ovsdb_jsonrpc_remote *remote;
 
-    shash_init(statuses);
+    memset(status, 0, sizeof *status);
 
-    SHASH_FOR_EACH (node, &svr->remotes) {
-        const struct ovsdb_jsonrpc_remote *remote = node->data;
-
-        ovsdb_jsonrpc_session_get_status(remote, statuses);
-    }
+    remote = shash_find_data(&svr->remotes, target);
+    return remote && ovsdb_jsonrpc_session_get_status(remote, status);
 }
 
 /* Forces all of the JSON-RPC sessions managed by 'svr' to disconnect and
@@ -439,32 +444,19 @@ ovsdb_jsonrpc_session_set_all_options(
     }
 }
 
-static void
+static bool
 ovsdb_jsonrpc_session_get_status(const struct ovsdb_jsonrpc_remote *remote,
-                                 struct shash *shash)
+                                 struct ovsdb_jsonrpc_remote_status *status)
 {
     const struct ovsdb_jsonrpc_session *s;
     const struct jsonrpc_session *js;
-    const char *name;
-    struct ovsdb_jsonrpc_remote_status *status;
     struct reconnect_stats rstats;
 
-    /* We only look at the first session in the list. There should be only one
-     * node in the list for outbound connections. We don't track status for
-     * each individual inbound connection if someone configures the DB that
-     * way. Since outbound connections are the norm, this is fine. */
     if (list_is_empty(&remote->sessions)) {
-        return;
+        return false;
     }
     s = CONTAINER_OF(remote->sessions.next, struct ovsdb_jsonrpc_session, node);
     js = s->js;
-    if (!js) {
-        return;
-    }
-    name = jsonrpc_session_get_name(js);
-
-    status = xzalloc(sizeof *status);
-    shash_add(shash, name, status);
 
     status->is_connected = jsonrpc_session_is_connected(js);
     status->last_error = jsonrpc_session_get_status(js);
@@ -475,6 +467,8 @@ ovsdb_jsonrpc_session_get_status(const struct ovsdb_jsonrpc_remote *remote,
         ? UINT_MAX : rstats.msec_since_connect / 1000;
     status->sec_since_disconnect = rstats.msec_since_disconnect == UINT_MAX
         ? UINT_MAX : rstats.msec_since_disconnect / 1000;
+
+    return true;
 }
 
 static const char *

diff --git a/ovsdb/jsonrpc-server.h b/ovsdb/jsonrpc-server.h
index dc78635..1d612cd 100644 (file)
--- a/ovsdb/jsonrpc-server.h
+++ b/ovsdb/jsonrpc-server.h
@@ -1,4 +1,4 @@
-/* Copyright (c) 2009, 2010 Nicira Networks
+/* Copyright (c) 2009, 2010, 2011 Nicira Networks
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -42,9 +42,9 @@ struct ovsdb_jsonrpc_remote_status {
     unsigned int sec_since_disconnect;
     bool is_connected;
 };
-void ovsdb_jsonrpc_server_get_remote_status(
-    const struct ovsdb_jsonrpc_server *,
-    struct shash * /* of 'struct ovsdb_jsonrpc_remote_status' */ );
+bool ovsdb_jsonrpc_server_get_remote_status(
+    const struct ovsdb_jsonrpc_server *, const char *target,
+    struct ovsdb_jsonrpc_remote_status *);
 
 void ovsdb_jsonrpc_server_reconnect(struct ovsdb_jsonrpc_server *);
 

diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
index 9c01c4e..e2fb958 100644 (file)
--- a/ovsdb/ovsdb-server.c
+++ b/ovsdb/ovsdb-server.c
@@ -463,12 +463,12 @@ query_db_remotes(const char *name, const struct ovsdb *db,
 
 static void
 update_remote_row(const struct ovsdb_row *row, struct ovsdb_txn *txn,
-                  const struct shash *statuses)
+                  const struct ovsdb_jsonrpc_server *jsonrpc)
 {
+    struct ovsdb_jsonrpc_remote_status status;
     struct ovsdb_row *rw_row;
     const char *target;
-    const struct ovsdb_jsonrpc_remote_status *status;
-    char *keys[4], *values[4];
+    char *keys[8], *values[8];
     size_t n = 0;
 
     /* Get the "target" (protocol/host/port) spec. */
@@ -476,43 +476,36 @@ update_remote_row(const struct ovsdb_row *row, struct ovsdb_txn *txn,
         /* Bad remote spec or incorrect schema. */
         return;
     }
-
-    /* Prepare to modify this row. */
     rw_row = ovsdb_txn_row_modify(txn, row);
-
-    /* Find status information for this target. */
-    status = shash_find_data(statuses, target);
-    if (!status) {
-        /* Should never happen, but just in case... */
-        return;
-    }
+    ovsdb_jsonrpc_server_get_remote_status(jsonrpc, target, &status);
 
     /* Update status information columns. */
+    write_bool_column(rw_row, "is_connected", status.is_connected);
 
-    write_bool_column(rw_row, "is_connected",
-                      status->is_connected);
-
-    keys[n] = xstrdup("state");
-    values[n++] = xstrdup(status->state);
-    if (status->sec_since_connect != UINT_MAX) {
+    if (status.state) {
+        keys[n] = xstrdup("state");
+        values[n++] = xstrdup(status.state);
+    }
+    if (status.sec_since_connect != UINT_MAX) {
         keys[n] = xstrdup("sec_since_connect");
-        values[n++] = xasprintf("%u", status->sec_since_connect);
+        values[n++] = xasprintf("%u", status.sec_since_connect);
     }
-    if (status->sec_since_disconnect != UINT_MAX) {
+    if (status.sec_since_disconnect != UINT_MAX) {
         keys[n] = xstrdup("sec_since_disconnect");
-        values[n++] = xasprintf("%u", status->sec_since_disconnect);
+        values[n++] = xasprintf("%u", status.sec_since_disconnect);
     }
-    if (status->last_error) {
+    if (status.last_error) {
         keys[n] = xstrdup("last_error");
         values[n++] =
-            xstrdup(ovs_retval_to_string(status->last_error));
+            xstrdup(ovs_retval_to_string(status.last_error));
     }
     write_string_string_column(rw_row, "status", keys, values, n);
 }
 
 static void
 update_remote_rows(const struct ovsdb *db, struct ovsdb_txn *txn,
-                   const char *remote_name, const struct shash *statuses)
+                   const char *remote_name,
+                   const struct ovsdb_jsonrpc_server *jsonrpc)
 {
     const struct ovsdb_table *table, *ref_table;
     const struct ovsdb_column *column;
@@ -542,7 +535,7 @@ update_remote_rows(const struct ovsdb *db, struct ovsdb_txn *txn,
 
             ref_row = ovsdb_table_get_row(ref_table, &datum->keys[i].uuid);
             if (ref_row) {
-                update_remote_row(ref_row, txn, statuses);
+                update_remote_row(ref_row, txn, jsonrpc);
             }
         }
     }
@@ -553,20 +546,16 @@ update_remote_status(const struct ovsdb_jsonrpc_server *jsonrpc,
                      const struct sset *remotes, struct ovsdb *db)
 {
     static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
-    struct shash statuses;
     struct ovsdb_txn *txn;
     const bool durable_txn = false;
     struct ovsdb_error *error;
     const char *remote;
 
-    /* Get status of current connections. */
-    ovsdb_jsonrpc_server_get_remote_status(jsonrpc, &statuses);
-
     txn = ovsdb_txn_create(db);
 
     /* Iterate over --remote arguments given on command line. */
     SSET_FOR_EACH (remote, remotes) {
-        update_remote_rows(db, txn, remote, &statuses);
+        update_remote_rows(db, txn, remote, jsonrpc);
     }
 
     error = ovsdb_txn_commit(txn, durable_txn);
@@ -574,8 +563,6 @@ update_remote_status(const struct ovsdb_jsonrpc_server *jsonrpc,
         VLOG_ERR_RL(&rl, "Failed to update remote status: %s",
                     ovsdb_error_to_string(error));
     }
-
-    shash_destroy_free_data(&statuses);
 }
 
 /* Reconfigures ovsdb-server based on information in the database. */