[libvirt] [PATCHv3 2/4] nodeinfo: improve probing node cpu bitmap

[Eric Blake]

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 7a87f2b..a9cae52 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 8f96b8b..461b5dc 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 2eda846..73c6f51 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 254f191..57e6403 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"));
-- 
1.7.11.7