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, virNodeParseNode): Update callers. --- I'm still testing this, but wanted to get eyes on it now as I think it belongs in 1.0.0 if it passes my testing. Having libvirtd dump core on RHEL 5 is not nice. I'm not quite sure when the regression was introduced, but it was caused by our refactoring of code to parse different sysfs files. src/nodeinfo.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 35c5f96..ccf0682 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,7 +237,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) @@ -275,7 +276,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) @@ -297,7 +298,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", -1); # endif CPU_SET(core, &core_maps[sock]); -- 1.7.11.7