From 04850866d9d01e5e667328d0b13929b744defdb9 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Tue, 31 Mar 2009 14:45:02 -0700 Subject: [PATCH] vswitch: Do not use "xe" on XenServer, to avoid deadlock. 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 | 2 +- vswitchd/automake.mk | 4 +- vswitchd/bridge.c | 29 +++---- vswitchd/xe.c | 140 --------------------------------- vswitchd/xenserver.c | 90 +++++++++++++++++++++ vswitchd/{xe.h => xenserver.h} | 8 +- 6 files changed, 107 insertions(+), 166 deletions(-) delete mode 100644 vswitchd/xe.c create mode 100644 vswitchd/xenserver.c rename vswitchd/{xe.h => xenserver.h} (91%) diff --git a/lib/vlog-modules.def b/lib/vlog-modules.def index db100e88..778f1c7a 100644 --- a/lib/vlog-modules.def +++ b/lib/vlog-modules.def @@ -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" diff --git a/vswitchd/automake.mk b/vswitchd/automake.mk index ded5a52a..fa1e6eea 100644 --- a/vswitchd/automake.mk +++ b/vswitchd/automake.mk @@ -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 \ diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index c1218494..1798ea0b 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -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 index ca623417..00000000 --- a/vswitchd/xe.c +++ /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 . - * - * 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 -#include "xe.h" -#include -#include -#include -#include -#include -#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/xenserver.c b/vswitchd/xenserver.c new file mode 100644 index 00000000..7a8d255f --- /dev/null +++ b/vswitchd/xenserver.c @@ -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 . + * + * 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 +#include "xenserver.h" +#include +#include +#include +#include +#include +#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/xe.h b/vswitchd/xenserver.h similarity index 91% rename from vswitchd/xe.h rename to vswitchd/xenserver.h index deb6ede9..c69b133a 100644 --- a/vswitchd/xe.h +++ b/vswitchd/xenserver.h @@ -24,9 +24,9 @@ * delete this exception statement from your version. */ -#ifndef VSWITCHD_XE_H -#define VSWITCHD_XE_H 1 +#ifndef VSWITCHD_XENSERVER_H +#define VSWITCHD_XENSERVER_H 1 -char *xe_query_simple(const char *query); +const char *xenserver_get_host_uuid(void); -#endif /* xe.h */ +#endif /* xenserver.h */ -- 2.30.2