From ddf4a85db8e439ccdf9e45d2a62c5671819fb319 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Thu, 1 Nov 2012 16:20:09 -0600 Subject: [PATCH] nodeinfo: support kernels that lack socket information On RHEL 5, I was getting a segfault trying to start libvirtd, because we were failing virNodeParseSocket but not checking for errors, and then calling CPU_SET(-1, &sock_map) as a result. But if you don't have a topology/physical_package_id file, then you can just assume that the cpu belongs to socket 0. * src/nodeinfo.c (virNodeGetCpuValue): Change bool into default_value. (virNodeParseSocket): Allow for default value when file is missing, different from fatal error on reading file. (virNodeParseNode): Update call sites to fail on error. (cherry picked from commit 47976b484cfae6ff0dd9e6fcbd45d377aaeaa8f4) --- src/nodeinfo.c | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/src/nodeinfo.c b/src/nodeinfo.c index c0e60d8099..05b6d218f4 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -79,13 +79,14 @@ static int linuxNodeGetMemoryStats(FILE *meminfo, int *nparams); /* Return the positive decimal contents of the given - * DIR/cpu%u/FILE, or -1 on error. If MISSING_OK and the - * file could not be found, return 1 instead of an error; this is - * because some machines cannot hot-unplug cpu0, or because - * hot-unplugging is disabled. */ + * DIR/cpu%u/FILE, or -1 on error. If DEFAULT_VALUE is non-negative + * and the file could not be found, return that instead of an error; + * this is useful for machines that cannot hot-unplug cpu0, or where + * hot-unplugging is disabled, or where the kernel is too old + * to support NUMA cells, etc. */ static int virNodeGetCpuValue(const char *dir, unsigned int cpu, const char *file, - bool missing_ok) + int default_value) { char *path; FILE *pathfp; @@ -100,8 +101,8 @@ virNodeGetCpuValue(const char *dir, unsigned int cpu, const char *file, pathfp = fopen(path, "r"); if (pathfp == NULL) { - if (missing_ok && errno == ENOENT) - value = 1; + if (default_value >= 0 && errno == ENOENT) + value = default_value; else virReportSystemError(errno, _("cannot open %s"), path); goto cleanup; @@ -174,7 +175,7 @@ static int virNodeParseSocket(const char *dir, unsigned int cpu) { int ret = virNodeGetCpuValue(dir, cpu, "topology/physical_package_id", - false); + 0); # if defined(__powerpc__) || \ defined(__powerpc64__) || \ defined(__s390__) || \ @@ -236,14 +237,15 @@ virNodeParseNode(const char *node, int *sockets, int *cores, int *threads) if (sscanf(cpudirent->d_name, "cpu%u", &cpu) != 1) continue; - if ((online = virNodeGetCpuValue(node, cpu, "online", true)) < 0) + if ((online = virNodeGetCpuValue(node, cpu, "online", 1)) < 0) goto cleanup; if (!online) continue; /* Parse socket */ - sock = virNodeParseSocket(node, cpu); + if ((sock = virNodeParseSocket(node, cpu)) < 0) + goto cleanup; CPU_SET(sock, &sock_map); if (sock > sock_max) @@ -275,7 +277,7 @@ virNodeParseNode(const char *node, int *sockets, int *cores, int *threads) if (sscanf(cpudirent->d_name, "cpu%u", &cpu) != 1) continue; - if ((online = virNodeGetCpuValue(node, cpu, "online", true)) < 0) + if ((online = virNodeGetCpuValue(node, cpu, "online", 1)) < 0) goto cleanup; if (!online) @@ -284,7 +286,8 @@ virNodeParseNode(const char *node, int *sockets, int *cores, int *threads) processors++; /* Parse socket */ - sock = virNodeParseSocket(node, cpu); + if ((sock = virNodeParseSocket(node, cpu)) < 0) + goto cleanup; if (!CPU_ISSET(sock, &sock_map)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("CPU socket topology has changed")); @@ -297,7 +300,7 @@ virNodeParseNode(const char *node, int *sockets, int *cores, int *threads) /* logical cpu is equivalent to a core on s390 */ core = cpu; # else - core = virNodeGetCpuValue(node, cpu, "topology/core_id", false); + core = virNodeGetCpuValue(node, cpu, "topology/core_id", 0); # endif CPU_SET(core, &core_maps[sock]);