From 2f4c5338a6a7298e8bc4e1843a12318c512b32c1 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Wed, 24 Oct 2012 16:43:26 -0600 Subject: [PATCH] nodeinfo: improve probing node cpu bitmap Callers should not need to know what the name of the file to be read in the Linux-specific version of nodeGetCPUmap; furthermore, qemu cares about online cpus, not present cpus, when determining which cpus to skip. While at it, I fixed the fact that we were computing the maximum online cpu id by doing a slow iteration, when what we really want to know is the max available cpu. * src/nodeinfo.h (nodeGetCPUmap): Rename... (nodeGetCPUBitmap): ...and simplify signature. * src/nodeinfo.c (linuxParseCPUmax): New function. (linuxParseCPUmap): Simplify and alter signature. (nodeGetCPUBitmap): Change implementation. * src/libvirt_private.syms (nodeinfo.h): Reflect rename. * src/qemu/qemu_driver.c (qemuDomainGetPercpuStats): Update caller. --- src/libvirt_private.syms | 2 +- src/nodeinfo.c | 76 ++++++++++++++++++++++++++-------------- src/nodeinfo.h | 5 ++- src/qemu/qemu_driver.c | 6 ++-- 4 files changed, 56 insertions(+), 33 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 7a87f2b58d..a9cae523d2 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -907,7 +907,7 @@ virNodeDeviceObjUnlock; # nodeinfo.h nodeCapsInitNUMA; -nodeGetCPUmap; +nodeGetCPUBitmap; nodeGetCPUStats; nodeGetCellsFreeMemory; nodeGetFreeMemory; diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 8f96b8b2f6..461b5dc638 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -755,34 +755,55 @@ cleanup: return ret; } + +/* Determine the maximum cpu id from a Linux sysfs cpu/present file. */ +static int +linuxParseCPUmax(const char *path) +{ + char *str = NULL; + char *tmp; + int ret = -1; + + if (virFileReadAll(path, 5 * VIR_DOMAIN_CPUMASK_LEN, &str) < 0) { + virReportOOMError(); + goto cleanup; + } + + tmp = str; + do { + if (virStrToLong_i(tmp, &tmp, 10, &ret) < 0 || + !strchr(",-\n", *tmp)) { + virReportError(VIR_ERR_NO_SUPPORT, + _("failed to parse %s"), path); + ret = -1; + goto cleanup; + } + } while (*tmp++ != '\n'); + ret++; + +cleanup: + VIR_FREE(str); + return ret; +} + /* - * Linux maintains cpu bit map. For example, if cpuid=5's flag is not set - * and max cpu is 7. The map file shows 0-4,6-7. This function parses - * it and returns cpumap. + * Linux maintains cpu bit map under cpu/online. For example, if + * cpuid=5's flag is not set and max cpu is 7, the map file shows + * 0-4,6-7. This function parses it and returns cpumap. */ static virBitmapPtr -linuxParseCPUmap(int *max_cpuid, const char *path) +linuxParseCPUmap(int max_cpuid, const char *path) { virBitmapPtr map = NULL; char *str = NULL; - int max_id = 0, i; if (virFileReadAll(path, 5 * VIR_DOMAIN_CPUMASK_LEN, &str) < 0) { virReportOOMError(); goto error; } - if (virBitmapParse(str, 0, &map, - VIR_DOMAIN_CPUMASK_LEN) < 0) { + if (virBitmapParse(str, 0, &map, max_cpuid) < 0) goto error; - } - - i = -1; - while ((i = virBitmapNextSetBit(map, i)) >= 0) { - max_id = i; - } - - *max_cpuid = max_id; VIR_FREE(str); return map; @@ -929,21 +950,24 @@ int nodeGetMemoryStats(virConnectPtr conn ATTRIBUTE_UNUSED, } virBitmapPtr -nodeGetCPUmap(virConnectPtr conn ATTRIBUTE_UNUSED, - int *max_id ATTRIBUTE_UNUSED, - const char *mapname ATTRIBUTE_UNUSED) +nodeGetCPUBitmap(virConnectPtr conn ATTRIBUTE_UNUSED, + int *max_id ATTRIBUTE_UNUSED) { #ifdef __linux__ - char *path; virBitmapPtr cpumap; - - if (virAsprintf(&path, SYSFS_SYSTEM_PATH "/cpu/%s", mapname) < 0) { - virReportOOMError(); + int present; + + present = linuxParseCPUmax(SYSFS_SYSTEM_PATH "/cpu/present"); + /* XXX should we also work on older kernels, like RHEL5, that lack + * cpu/present and cpu/online files? Those kernels also lack cpu + * hotplugging, so it would be a matter of finding the largest + * cpu/cpuNN directory, and creating a map that size with all bits + * set. */ + if (present < 0) return NULL; - } - - cpumap = linuxParseCPUmap(max_id, path); - VIR_FREE(path); + cpumap = linuxParseCPUmap(present, SYSFS_SYSTEM_PATH "/cpu/online"); + if (max_id && cpumap) + *max_id = present; return cpumap; #else virReportError(VIR_ERR_NO_SUPPORT, "%s", diff --git a/src/nodeinfo.h b/src/nodeinfo.h index 2eda8460c8..73c6f514c7 100644 --- a/src/nodeinfo.h +++ b/src/nodeinfo.h @@ -46,9 +46,8 @@ int nodeGetCellsFreeMemory(virConnectPtr conn, int maxCells); unsigned long long nodeGetFreeMemory(virConnectPtr conn); -virBitmapPtr nodeGetCPUmap(virConnectPtr conn, - int *max_id, - const char *mapname); +virBitmapPtr nodeGetCPUBitmap(virConnectPtr conn, + int *max_id); int nodeGetMemoryParameters(virConnectPtr conn, virTypedParameterPtr params, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 254f191d69..57e640360d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13615,8 +13615,8 @@ qemuDomainGetPercpuStats(virDomainPtr domain, if (nparams == 0 && ncpus != 0) return QEMU_NB_PER_CPU_STAT_PARAM; - /* To parse account file, we need "present" cpu map. */ - map = nodeGetCPUmap(domain->conn, &max_id, "present"); + /* To parse account file, we need bitmap of online cpus. */ + map = nodeGetCPUBitmap(domain->conn, &max_id); if (!map) return rv; @@ -13681,7 +13681,7 @@ qemuDomainGetPercpuStats(virDomainPtr domain, goto cleanup; /* Check that the mapping of online cpus didn't change mid-parse. */ - map2 = nodeGetCPUmap(domain->conn, &max_id, "present"); + map2 = nodeGetCPUBitmap(domain->conn, &max_id); if (!map2 || !virBitmapEqual(map, map2)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("the set of online cpus changed while reading")); -- GitLab