vswitch: Do not use "xe" on XenServer, to avoid deadlock.
authorBen Pfaff <blp@nicira.com>
Tue, 31 Mar 2009 21:45:02 +0000 (14:45 -0700)
committerBen Pfaff <blp@nicira.com>
Tue, 31 Mar 2009 21:45:02 +0000 (14:45 -0700)
Commit 4ab581b949, "vswitch: Fix duplicated DPIDs observed under
XenServer," changed vswitchd to use the "xe" program to obtain the host
UUID and network name under XenServer.  Unfortunately, on XenServer pool
slaves this causes a deadlock, because on such machines "xe" always
attempts to contact the master to perform database queries.  Since vswitchd
is the entity that must set up the flow that "xe" initiates as part of
that connection, this will deadlock and fail.

Instead, get the host UUID from /etc/xensource-inventory and hash it with
the bridge name.  Discussion with the NOX folks indicates that this should
provide sufficient stability and uniqueness.

lib/vlog-modules.def
vswitchd/automake.mk
vswitchd/bridge.c
vswitchd/xe.c [deleted file]
vswitchd/xe.h [deleted file]
vswitchd/xenserver.c [new file with mode: 0644]
vswitchd/xenserver.h [new file with mode: 0644]

index db100e8828847e66f9c3de574fce7af1dd6cec6b..778f1c7a9b76b14650f05dd80708eb838b5e5f41 100644 (file)
@@ -53,7 +53,7 @@ VLOG_MODULE(vlog)
 VLOG_MODULE(vlog_socket)
 VLOG_MODULE(wcelim)
 VLOG_MODULE(vswitchd)
-VLOG_MODULE(xe)
+VLOG_MODULE(xenserver)
 
 #ifdef HAVE_EXT
 #include "ext/vlogext-modules.def"
index ded5a52a0d5900773897799b94eac61c9cf9de6c..fa1e6eeaba8212ee8088663f5e32d3dbf4aa1df8 100644 (file)
@@ -12,8 +12,8 @@ vswitchd_vswitchd_SOURCES = \
        vswitchd/mgmt.c \
        vswitchd/mgmt.h \
        vswitchd/vswitchd.c \
-       vswitchd/xe.c \
-       vswitchd/xe.h
+       vswitchd/xenserver.c \
+       vswitchd/xenserver.h
 vswitchd_vswitchd_LDADD = \
        secchan/libsecchan.a \
        lib/libopenflow.a \
index c1218494871cbb332ca233f3c906a4814c2b2f6b..1798ea0b07de0369e9fe57982883ce87aa2ebd06 100644 (file)
@@ -64,7 +64,7 @@
 #include "util.h"
 #include "vconn.h"
 #include "vconn-ssl.h"
-#include "xe.h"
+#include "xenserver.h"
 #include "xtoxll.h"
 
 #define THIS_MODULE VLM_bridge
@@ -627,32 +627,23 @@ bridge_pick_datapath_id(struct bridge *br,
          * network devices on it at all, is more difficult because it has no
          * natural unique identifier at all.
          *
-         * When the host is a XenServer, we handle this case by asking "xe" for
-         * the internal bridge's plaintext name (e.g. "Internal Network 1") and
-         * hashing that together with the XenServer's host UUID.
+         * When the host is a XenServer, we handle this case by hashing the
+         * host's UUID with the name of the bridge.  Names of bridges are
+         * persistent across XenServer reboots, although they can be reused if
+         * an internal network is destroyed and then a new one is later
+         * created, so this is fairly effective.
          *
          * When the host is not a XenServer, we punt by using a random MAC
          * address on each run.
          */
-        char *host_uuid;
-        char *network_query, *network_name;
-
-        host_uuid = xe_query_simple("host-list params=uuid");
-        network_query = xasprintf("network-list 'bridge=%s' params=name-label",
-                                  br->name);
-        network_name = xe_query_simple(network_query);
-        free(network_query);
-
-        if (host_uuid && network_name) {
-            char *combined = xasprintf("%s%s", host_uuid, network_name);
+        const char *host_uuid = xenserver_get_host_uuid();
+        if (host_uuid) {
+            char *combined = xasprintf("%s,%s", host_uuid, br->name);
+            printf("%s\n", combined);
             dpid = dpid_from_hash(combined, strlen(combined));
             free(combined);
-            free(host_uuid);
-            free(network_name);
             return dpid;
         }
-        free(host_uuid);
-        free(network_name);
     }
 
     return eth_addr_to_uint64(bridge_ea);
diff --git a/vswitchd/xe.c b/vswitchd/xe.c
deleted file mode 100644 (file)
index ca62341..0000000
+++ /dev/null
@@ -1,140 +0,0 @@
-/* Copyright (c) 2009 Nicira Networks
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- *
- * In addition, as a special exception, Nicira Networks gives permission
- * to link the code of its release of vswitchd with the OpenSSL project's
- * "OpenSSL" library (or with modified versions of it that use the same
- * license as the "OpenSSL" library), and distribute the linked
- * executables.  You must obey the GNU General Public License in all
- * respects for all of the code used other than "OpenSSL".  If you modify
- * this file, you may extend this exception to your version of the file,
- * but you are not obligated to do so.  If you do not wish to do so,
- * delete this exception statement from your version.
- */
-
-#include <config.h>
-#include "xe.h"
-#include <ctype.h>
-#include <errno.h>
-#include <stdlib.h>
-#include <string.h>
-#include <unistd.h>
-#include "dynamic-string.h"
-#include "process.h"
-
-#include "vlog.h"
-#define THIS_MODULE VLM_xe
-
-static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
-
-static const char *find_xe(void);
-
-char *
-xe_query_simple(const char *query)
-{
-    const char *binary;
-    char *cmd = NULL;
-    FILE *stream = NULL;
-    struct ds line = DS_EMPTY_INITIALIZER;
-    char *answer = NULL;
-    const char *p;
-    size_t len;
-    int status;
-
-    binary = find_xe();
-    if (!binary) {
-        goto exit;
-    }
-
-    cmd = xasprintf("%s %s", binary, query);
-    stream = popen(cmd, "r");
-    if (!stream) {
-        VLOG_ERR_RL(&rl, "failed to start \"%s\": %s", cmd, strerror(errno));
-        goto exit;
-    }
-
-    if (ds_get_line(&line, stream)) {
-        if (ferror(stream)) {
-            VLOG_ERR_RL(&rl, "error reading output from \"%s\": %s",
-                        cmd, strerror(errno));
-        } else {
-            VLOG_ERR_RL(&rl, "end of file reading output from \"%s\"", cmd);
-        }
-        goto exit;
-    }
-
-    p = strchr(ds_cstr(&line), ':');
-    if (!p) {
-        VLOG_ERR_RL(&rl, "parse error reading output from \"%s\" "
-                    "(\"%s\" does not contain a colon)", cmd, line.string);
-        goto exit;
-    }
-
-    p++;
-    while (isspace((unsigned char) *p)) {
-        p++;
-    }
-    len = strlen(p);
-    while (len > 0 && isspace((unsigned char) p[len - 1])) {
-        len--;
-    }
-    answer = xmemdup0(p, len);
-
-    status = pclose(stream);
-    stream = NULL;
-    if (status == -1) {
-        VLOG_WARN_RL(&rl, "pclose of \"%s\" command failed: %s",
-                    cmd, strerror(errno));
-    } else if (status) {
-        char *msg = process_status_msg(status);
-        VLOG_WARN_RL(&rl, "pclose(\"%s\") reported subprocess failure: %s",
-                     cmd, msg);
-        free(msg);
-    }
-
-exit:
-    if (answer) {
-        VLOG_DBG("\"%s\" answered \"%s\"", query, answer);
-    }
-    free(cmd);
-    if (stream) {
-        pclose(stream);
-    }
-    ds_destroy(&line);
-    return answer;
-}
-
-static const char *
-find_xe(void)
-{
-    static char *xe_binary;
-    static bool searched = false;
-    if (!searched) {
-        searched = true;
-        if (getenv("XE")) {
-            xe_binary = getenv("XE");
-            VLOG_INFO("using xe binary %s from XE",
-                      xe_binary);
-        } else {
-            xe_binary = process_search_path("xe");
-            if (xe_binary) {
-                VLOG_INFO("using xe binary %s (from PATH)", xe_binary);
-            } else {
-                VLOG_WARN("xe binary not found in PATH or XE");
-            }
-        }
-    }
-    return xe_binary;
-}
diff --git a/vswitchd/xe.h b/vswitchd/xe.h
deleted file mode 100644 (file)
index deb6ede..0000000
+++ /dev/null
@@ -1,32 +0,0 @@
-/* Copyright (c) 2009 Nicira Networks
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- *
- * In addition, as a special exception, Nicira Networks gives permission
- * to link the code of its release of vswitchd with the OpenSSL project's
- * "OpenSSL" library (or with modified versions of it that use the same
- * license as the "OpenSSL" library), and distribute the linked
- * executables.  You must obey the GNU General Public License in all
- * respects for all of the code used other than "OpenSSL".  If you modify
- * this file, you may extend this exception to your version of the file,
- * but you are not obligated to do so.  If you do not wish to do so,
- * delete this exception statement from your version.
- */
-
-#ifndef VSWITCHD_XE_H
-#define VSWITCHD_XE_H 1
-
-char *xe_query_simple(const char *query);
-
-#endif /* xe.h */
diff --git a/vswitchd/xenserver.c b/vswitchd/xenserver.c
new file mode 100644 (file)
index 0000000..7a8d255
--- /dev/null
@@ -0,0 +1,90 @@
+/* Copyright (c) 2009 Nicira Networks
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * In addition, as a special exception, Nicira Networks gives permission
+ * to link the code of its release of vswitchd with the OpenSSL project's
+ * "OpenSSL" library (or with modified versions of it that use the same
+ * license as the "OpenSSL" library), and distribute the linked
+ * executables.  You must obey the GNU General Public License in all
+ * respects for all of the code used other than "OpenSSL".  If you modify
+ * this file, you may extend this exception to your version of the file,
+ * but you are not obligated to do so.  If you do not wish to do so,
+ * delete this exception statement from your version.
+ */
+
+#include <config.h>
+#include "xenserver.h"
+#include <ctype.h>
+#include <errno.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include "dynamic-string.h"
+#include "process.h"
+
+#include "vlog.h"
+#define THIS_MODULE VLM_xenserver
+
+static char *
+read_host_uuid(void)
+{
+    static const char filename[] = "/etc/xensource-inventory";
+    char line[128];
+    FILE *file;
+
+    file = fopen(filename, "r");
+    if (!file) {
+        if (errno == ENOENT) {
+            VLOG_INFO("not running on a XenServer");
+        } else {
+            VLOG_INFO("%s: open: %s", filename, strerror(errno));
+        }
+        return NULL;
+    }
+
+    while (fgets(line, sizeof line, file)) {
+        static const char leader[] = "INSTALLATION_UUID='";
+        const int leader_len = strlen(leader);
+        const int uuid_len = 36;
+        static const char trailer[] = "'\n";
+        const int trailer_len = strlen(trailer);
+
+        if (strlen(line) == leader_len + uuid_len + trailer_len
+            && !memcmp(line, leader, leader_len)
+            && !memcmp(line + leader_len + uuid_len, trailer, trailer_len)) {
+            char *host_uuid = xmemdup0(line + leader_len, uuid_len);
+            VLOG_INFO("running on XenServer, host-uuid %s", host_uuid);
+            fclose(file);
+            return host_uuid;
+        }
+    }
+    fclose(file);
+    VLOG_ERR("%s: INSTALLATION_UUID not found", filename);
+    return NULL;
+}
+
+const char *
+xenserver_get_host_uuid(void)
+{
+    static char *host_uuid;
+    static bool inited;
+
+    if (!inited) {
+        host_uuid = read_host_uuid();
+        inited = true;
+    }
+    return host_uuid;
+}
+
diff --git a/vswitchd/xenserver.h b/vswitchd/xenserver.h
new file mode 100644 (file)
index 0000000..c69b133
--- /dev/null
@@ -0,0 +1,32 @@
+/* Copyright (c) 2009 Nicira Networks
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * In addition, as a special exception, Nicira Networks gives permission
+ * to link the code of its release of vswitchd with the OpenSSL project's
+ * "OpenSSL" library (or with modified versions of it that use the same
+ * license as the "OpenSSL" library), and distribute the linked
+ * executables.  You must obey the GNU General Public License in all
+ * respects for all of the code used other than "OpenSSL".  If you modify
+ * this file, you may extend this exception to your version of the file,
+ * but you are not obligated to do so.  If you do not wish to do so,
+ * delete this exception statement from your version.
+ */
+
+#ifndef VSWITCHD_XENSERVER_H
+#define VSWITCHD_XENSERVER_H 1
+
+const char *xenserver_get_host_uuid(void);
+
+#endif /* xenserver.h */