As pointed out by eblake, I made a real hash of the nodeinfo code with commit aa2f6f96ddd7a57011c3d25586d588100651feb2. This patch cleans it up: 1) Do more work at compile time instead of runtime (minor) 2) Properly handle the hex digits that come from /sys/devices/system/cpu/cpu*/topology/thread_siblings 3) Fix up some error paths that could cause SEGV 4) Used unsigned's for the cpu numbers (cpu -1 doesn't make any sense) Along with the recent patch from jdenemar to zero out the nodeinfo structure, I've re-tested this on the machines having the problems, and it seems to be good. Signed-off-by: Chris Lalancette <clalance(a)redhat.com&gt; --- src/nodeinfo.c | 45 +++++++++++++++++++++++++++++++-------------- 1 files changed, 31 insertions(+), 14 deletions(-) diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 1ee3709..3fabeec 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -63,15 +63,15 @@ int linuxNodeInfoCPUPopulate(virConnectPtr conn, FILE *cpuinfo, virNodeInfoPtr nodeinfo); -static unsigned long count_thread_siblings(int cpu) +static unsigned long count_thread_siblings(unsigned int cpu) { unsigned long ret = 0; - char *path = NULL; - FILE *pathfp = NULL; + char *path; + FILE *pathfp; char str[1024]; int i; - if (virAsprintf(&path, "%s/cpu%d/topology/thread_siblings", CPU_SYS_PATH, + if (virAsprintf(&path, CPU_SYS_PATH "/cpu%u/topology/thread_siblings", cpu) < 0) { virReportOOMError(); return 0; @@ -91,8 +91,12 @@ static unsigned long count_thread_siblings(int cpu) i = 0; while (str[i] != '\0') { - if (str[i] != '\n' && str[i] != ',') + if (c_isdigit(str[i])) ret += count_one_bits(str[i] - '0'); + else if (str[i] >= 'A' && str[i] <= 'F') + ret += count_one_bits(str[i] - 'A' + 10); + else if (str[i] >= 'a' && str[i] <= 'f') + ret += count_one_bits(str[i] - 'a' + 10); i++; } @@ -103,16 +107,16 @@ cleanup: return ret; } -static int parse_socket(int cpu) +static int parse_socket(unsigned int cpu) { - char *path = NULL; + char *path; FILE *pathfp; char socket_str[1024]; char *tmp; - int socket; + int socket = -1; - if (virAsprintf(&path, "%s/cpu%d/topology/physical_package_id", - CPU_SYS_PATH, cpu) < 0) { + if (virAsprintf(&path, CPU_SYS_PATH "/cpu%u/topology/physical_package_id", + cpu) < 0) { virReportOOMError(); return -1; } @@ -120,7 +124,8 @@ static int parse_socket(int cpu) pathfp = fopen(path, "r"); if (pathfp == NULL) { virReportSystemError(errno, _("cannot open %s"), path); - goto cleanup; + VIR_FREE(path); + return -1; } if (fgets(socket_str, sizeof(socket_str), pathfp) == NULL) { @@ -147,7 +152,7 @@ int linuxNodeInfoCPUPopulate(virConnectPtr conn, FILE *cpuinfo, char line[1024]; DIR *cpudir = NULL; struct dirent *cpudirent = NULL; - int cpu; + unsigned int cpu; unsigned long cur_threads; int socket; unsigned long long socket_mask = 0; @@ -157,7 +162,7 @@ int linuxNodeInfoCPUPopulate(virConnectPtr conn, FILE *cpuinfo, nodeinfo->nodes = nodeinfo->cores = 1; /* NB: It is impossible to fill our nodes, since cpuinfo - * has not knowledge of NUMA nodes */ + * has no knowledge of NUMA nodes */ /* NOTE: hyperthreads are ignored here; they are parsed out of /sys */ while (fgets(line, sizeof(line), cpuinfo) != NULL) { @@ -220,7 +225,7 @@ int linuxNodeInfoCPUPopulate(virConnectPtr conn, FILE *cpuinfo, return -1; } while ((cpudirent = readdir(cpudir))) { - if (sscanf(cpudirent->d_name, "cpu%d", &cpu) != 1) + if (sscanf(cpudirent->d_name, "cpu%u", &cpu) != 1) continue; socket = parse_socket(cpu); @@ -244,6 +249,18 @@ int linuxNodeInfoCPUPopulate(virConnectPtr conn, FILE *cpuinfo, closedir(cpudir); + /* there should always be at least one socket and one thread */ + if (nodeinfo->sockets == 0) { + nodeReportError(conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("no sockets found")); + return -1; + } + if (nodeinfo->threads == 0) { + nodeReportError(conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("no threads found")); + return -1; + } + return 0; } -- 1.6.6.1