diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 7e68880f76..cb047c40f4 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -58,10 +58,9 @@ #ifdef __linux__ # define CPUINFO_PATH "/proc/cpuinfo" -# define CPU_SYS_PATH "/sys/devices/system/cpu" +# define SYSFS_SYSTEM_PATH "/sys/devices/system" # define PROCSTAT_PATH "/proc/stat" # define MEMINFO_PATH "/proc/meminfo" -# define NODE_SYS_PATH "/sys/devices/system/node" # define LINUX_NB_CPU_STATS 4 # define LINUX_NB_MEMORY_STATS_ALL 4 @@ -69,7 +68,7 @@ /* NB, this is not static as we need to call it from the testsuite */ int linuxNodeInfoCPUPopulate(FILE *cpuinfo, - const char *sysfs_cpudir, + const char *sysfs_dir, virNodeInfoPtr nodeinfo); static int linuxNodeGetCPUStats(FILE *procstat, @@ -189,8 +188,43 @@ virNodeParseSocket(const char *dir, unsigned int cpu) return ret; } +static int +virNodeParseNode(const char *sysfs_dir) +{ + char *file = NULL; + char *possible = NULL; + char *tmp; + int ret = -1; + + if (virAsprintf(&file, "%s/node/possible", sysfs_dir) < 0) { + virReportOOMError(); + goto cleanup; + } + /* Assume that a missing node/possible file implies no NUMA + * support, and hence all cpus belong to the same node. */ + if (!virFileExists(file)) { + ret = 1; + goto cleanup; + } + if (virFileReadAll(file, 1024, &possible) < 0) + goto cleanup; + if (virStrToLong_i(possible, &tmp, 10, &ret) < 0 || + (*tmp == '-' && virStrToLong_i(tmp+1, &tmp, 10, &ret) < 0) || + *tmp != '\n') { + nodeReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to parse possible nodes '%s'"), possible); + goto cleanup; + } + ret++; + +cleanup: + VIR_FREE(file); + VIR_FREE(possible); + return ret; +} + int linuxNodeInfoCPUPopulate(FILE *cpuinfo, - const char *sysfs_cpudir, + const char *sysfs_dir, virNodeInfoPtr nodeinfo) { char line[1024]; @@ -201,21 +235,15 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo, cpu_set_t core_mask; cpu_set_t socket_mask; int online; + int ret = -1; + char *sysfs_cpudir = NULL; nodeinfo->cpus = 0; nodeinfo->mhz = 0; nodeinfo->cores = 0; - nodeinfo->nodes = 1; -# if HAVE_NUMACTL - if (numa_available() >= 0) - nodeinfo->nodes = numa_max_node() + 1; -# endif - - /* NB: It is impossible to fill our nodes, since cpuinfo - * has no knowledge of NUMA nodes */ - - /* NOTE: hyperthreads are ignored here; they are parsed out of /sys */ + /* Start with parsing /proc/cpuinfo; although it tends to have + * fewer details. Hyperthreads are ignored at this stage. */ while (fgets(line, sizeof(line), cpuinfo) != NULL) { # if defined(__x86_64__) || \ defined(__amd64__) || \ @@ -230,7 +258,7 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo, if (*buf != ':' || !buf[1]) { nodeReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("parsing cpuinfo cpu MHz")); - return -1; + goto cleanup; } if (virStrToLong_ui(buf+1, &p, 10, &ui) == 0 /* Accept trailing fractional part. */ @@ -249,7 +277,7 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo, if (*buf != ':' || !buf[1]) { nodeReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("parsing cpuinfo cpu MHz")); - return -1; + goto cleanup; } if (virStrToLong_ui(buf+1, &p, 10, &ui) == 0 /* Accept trailing fractional part. */ @@ -266,13 +294,17 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo, # endif } - /* OK, we've parsed clock speed out of /proc/cpuinfo. Get the core, socket - * thread and topology information from /sys + /* OK, we've parsed clock speed out of /proc/cpuinfo. Get the + * core, node, socket, thread and topology information from /sys */ + if (virAsprintf(&sysfs_cpudir, "%s/cpu", sysfs_dir) < 0) { + virReportOOMError(); + goto cleanup; + } cpudir = opendir(sysfs_cpudir); if (cpudir == NULL) { virReportSystemError(errno, _("cannot opendir %s"), sysfs_cpudir); - return -1; + goto cleanup; } CPU_ZERO(&core_mask); @@ -285,7 +317,7 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo, online = virNodeGetCpuValue(sysfs_cpudir, cpu, "online", true); if (online < 0) { closedir(cpudir); - return -1; + goto cleanup; } if (!online) continue; @@ -308,7 +340,7 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo, cur_threads = virNodeCountThreadSiblings(sysfs_cpudir, cpu); if (cur_threads == 0) { closedir(cpudir); - return -1; + goto cleanup; } if (cur_threads > nodeinfo->threads) nodeinfo->threads = cur_threads; @@ -317,26 +349,32 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo, virReportSystemError(errno, _("problem reading %s"), sysfs_cpudir); closedir(cpudir); - return -1; + goto cleanup; + } + if (closedir(cpudir) < 0) { + virReportSystemError(errno, + _("problem closing %s"), sysfs_cpudir); + goto cleanup; } - closedir(cpudir); + if ((nodeinfo->nodes = virNodeParseNode(sysfs_dir)) <= 0) + goto cleanup; - /* there should always be at least one cpu, socket and one thread */ + /* There should always be at least one cpu, socket, node, and thread. */ if (nodeinfo->cpus == 0) { nodeReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("no CPUs found")); - return -1; + goto cleanup; } if (nodeinfo->sockets == 0) { nodeReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("no sockets found")); - return -1; + goto cleanup; } if (nodeinfo->threads == 0) { nodeReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("no threads found")); - return -1; + goto cleanup; } /* nodeinfo->sockets is supposed to be a number of sockets per NUMA node, @@ -349,7 +387,11 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo, else nodeinfo->nodes = 1; - return 0; + ret = 0; + +cleanup: + VIR_FREE(sysfs_cpudir); + return ret; } # define TICK_TO_NSEC (1000ull * 1000ull * 1000ull / sysconf(_SC_CLK_TCK)) @@ -620,7 +662,7 @@ int nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, virNodeInfoPtr nodeinfo) { return -1; } - ret = linuxNodeInfoCPUPopulate(cpuinfo, CPU_SYS_PATH, nodeinfo); + ret = linuxNodeInfoCPUPopulate(cpuinfo, SYSFS_SYSTEM_PATH, nodeinfo); if (ret < 0) goto cleanup; @@ -707,8 +749,8 @@ int nodeGetMemoryStats(virConnectPtr conn ATTRIBUTE_UNUSED, } # endif - if (virAsprintf(&meminfo_path, "%s/node%d/meminfo", - NODE_SYS_PATH, cellNum) < 0) { + if (virAsprintf(&meminfo_path, "%s/node/node%d/meminfo", + SYSFS_SYSTEM_PATH, cellNum) < 0) { virReportOOMError(); return -1; } @@ -743,7 +785,7 @@ nodeGetCPUmap(virConnectPtr conn ATTRIBUTE_UNUSED, char *path; char *cpumap; - if (virAsprintf(&path, CPU_SYS_PATH "/%s", mapname) < 0) { + if (virAsprintf(&path, SYSFS_SYSTEM_PATH "/cpu/%s", mapname) < 0) { virReportOOMError(); return NULL; } diff --git a/tests/nodeinfotest.c b/tests/nodeinfotest.c index 74bf811c68..872a526bdc 100644 --- a/tests/nodeinfotest.c +++ b/tests/nodeinfotest.c @@ -26,12 +26,12 @@ main(void) #else extern int linuxNodeInfoCPUPopulate(FILE *cpuinfo, - char *sysfs_cpuinfo, + char *sysfs_dir, virNodeInfoPtr nodeinfo); static int linuxTestCompareFiles(const char *cpuinfofile, - char *sysfs_cpuinfo, + char *sysfs_dir, const char *outputfile) { int ret = -1; @@ -48,7 +48,7 @@ linuxTestCompareFiles(const char *cpuinfofile, goto fail; memset(&nodeinfo, 0, sizeof(nodeinfo)); - if (linuxNodeInfoCPUPopulate(cpuinfo, sysfs_cpuinfo, &nodeinfo) < 0) { + if (linuxNodeInfoCPUPopulate(cpuinfo, sysfs_dir, &nodeinfo) < 0) { if (virTestGetDebug()) { virErrorPtr error = virSaveLastError(); if (error && error->code != VIR_ERR_OK) @@ -60,21 +60,13 @@ linuxTestCompareFiles(const char *cpuinfofile, } VIR_FORCE_FCLOSE(cpuinfo); - /* 'nodes' is filled using libnuma.so from current machine - * topology, which makes it unsuitable for the test suite - * so blank it to a predictable value */ - nodeinfo.nodes = 1; - if (virAsprintf(&actualData, "CPUs: %u, MHz: %u, Nodes: %u, Cores: %u\n", nodeinfo.cpus, nodeinfo.mhz, nodeinfo.nodes, nodeinfo.cores) < 0) goto fail; if (STRNEQ(actualData, expectData)) { - if (getenv("DEBUG_TESTS")) { - printf("Expect %d '%s'\n", (int)strlen(expectData), expectData); - printf("Actual %d '%s'\n", (int)strlen(actualData), actualData); - } + virtTestDifference(stderr, expectData, actualData); goto fail; } @@ -92,34 +84,31 @@ linuxTestNodeInfo(const void *data) { int result = -1; char *cpuinfo = NULL; - char *sysfs_cpuinfo = NULL; + char *sysfs_dir = NULL; char *output = NULL; + const char *test = data; + const char *arch = "x86"; # if defined(__powerpc__) || \ defined(__powerpc64__) - if (virAsprintf(&sysfs_cpuinfo, "%s/nodeinfodata/linux-%s/cpu/", - abs_srcdir, (const char*)data) < 0 || - virAsprintf(&cpuinfo, "%s/nodeinfodata/linux-%s-ppc.cpuinfo", - abs_srcdir, (const char*)data) < 0 || - virAsprintf(&output, "%s/nodeinfodata/linux-%s-cpu-ppc-output.txt", - abs_srcdir, (const char*)data) < 0) { -# else - if (virAsprintf(&sysfs_cpuinfo, "%s/nodeinfodata/linux-%s/cpu/", - abs_srcdir, (const char*)data) < 0 || - virAsprintf(&cpuinfo, "%s/nodeinfodata/linux-%s-x86.cpuinfo", - abs_srcdir, (const char*)data) < 0 || - virAsprintf(&output, "%s/nodeinfodata/linux-%s-cpu-x86-output.txt", - abs_srcdir, (const char*)data) < 0) { + arch = "ppc"; # endif + + if (virAsprintf(&sysfs_dir, "%s/nodeinfodata/linux-%s", + abs_srcdir, test) < 0 || + virAsprintf(&cpuinfo, "%s/nodeinfodata/linux-%s-%s.cpuinfo", + abs_srcdir, test, arch) < 0 || + virAsprintf(&output, "%s/nodeinfodata/linux-%s-cpu-%s-output.txt", + abs_srcdir, test, arch) < 0) { goto cleanup; } - result = linuxTestCompareFiles(cpuinfo, sysfs_cpuinfo, output); + result = linuxTestCompareFiles(cpuinfo, sysfs_dir, output); cleanup: VIR_FREE(cpuinfo); VIR_FREE(output); - VIR_FREE(sysfs_cpuinfo); + VIR_FREE(sysfs_dir); return result; }