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.

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 7a87f2b58ddb9ee61bdadc0910c898fe188cfe9b..a9cae523d211aa91388f51deebd5e3bc3678d713 100644 (file)
--- 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 8f96b8b2f664274c10f7266d5d6c8cbe138e5cbb..461b5dc638862f7a260566dd37851f6943a6c500 100644 (file)
--- 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 2eda8460c8f1ee4a3b62009d92b3c6f22683b723..73c6f514c7829ddadd046581f76d635a4528031e 100644 (file)
--- 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 254f191d690589b77a8e75b9bc0748e2eca0d12d..57e640360d21d11c3a850390f08c735cdc9805a8 100644 (file)
--- 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"));