1:30 p.m.
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>
---
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