[libvirt] [PATCH] nodeinfo: skip offline CPUs

[Eric Blake]

https://bugzilla.redhat.com/622515 - When hot-unplugging CPUs,
libvirt failed to start a guest that had been pinned to CPUs that
were still online, because it was failing to read status from
unrelated offline CPUs.

Tested on a dual-core laptop, where I also discovered that, per
http://www.cyberciti.biz/files/linux-kernel/Documentation/cpu-hotplug.txt,
/sys/devices/system/cpu/cpu0/online does not exist on systems where it
cannot be hot-unplugged.

* src/nodeinfo.c (linuxNodeInfoCPUPopulate): Ignore CPUs that are
currently offline.  Detect readdir failure.
(parse_socket): Move guts...
(get_cpu_value): ...to new function, shared with...
(cpu_online): New function.
---
 src/nodeinfo.c |  109 +++++++++++++++++++++++++++++++++++++------------------
 1 files changed, 73 insertions(+), 36 deletions(-)

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 5ec1bcf..6286aa2 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -23,6 +23,7 @@

 #include <config.h>

+#include <stdbool.h>
 #include <stdio.h>
 #include <string.h>
 #include <stdlib.h>
@@ -60,6 +61,58 @@
 int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
                              virNodeInfoPtr nodeinfo);

+/* Return the positive decimal contents of the given
+ * CPU_SYS_PATH/cpu%u/FILE, or -1 on error.  If MISSING_OK and the
+ * file could not be found, return 1 instead of an error; this is
+ * because some machines cannot hot-unplug cpu0.  */
+static int get_cpu_value(unsigned int cpu, const char *file, bool missing_ok)
+{
+    char *path;
+    FILE *pathfp;
+    char value_str[1024];
+    char *tmp;
+    int value = -1;
+
+    if (virAsprintf(&path, CPU_SYS_PATH "/cpu%u/%s", cpu, file) < 0) {
+        virReportOOMError();
+        return -1;
+    }
+
+    pathfp = fopen(path, "r");
+    if (pathfp == NULL) {
+        if (missing_ok && errno == ENOENT)
+            value = 1;
+        else
+            virReportSystemError(errno, _("cannot open %s"), path);
+        goto cleanup;
+    }
+
+    if (fgets(value_str, sizeof(value_str), pathfp) == NULL) {
+        virReportSystemError(errno, _("cannot read from %s"), path);
+        goto cleanup;
+    }
+    if (virStrToLong_i(value_str, &tmp, 10, &value) < 0) {
+        nodeReportError(VIR_ERR_INTERNAL_ERROR,
+                        _("could not convert '%s' to an integer"),
+                        value_str);
+        goto cleanup;
+    }
+
+cleanup:
+    if (pathfp)
+        fclose(pathfp);
+    VIR_FREE(path);
+
+    return value;
+}
+
+/* Check if CPU is online via CPU_SYS_PATH/cpu%u/online.  Return 1 if online,
+   0 if offline, and -1 on error.  */
+static int cpu_online(unsigned int cpu)
+{
+    return get_cpu_value(cpu, "online", cpu == 0);
+}
+
 static unsigned long count_thread_siblings(unsigned int cpu)
 {
     unsigned long ret = 0;
@@ -106,41 +159,7 @@ cleanup:

 static int parse_socket(unsigned int cpu)
 {
-    char *path;
-    FILE *pathfp;
-    char socket_str[1024];
-    char *tmp;
-    int socket = -1;
-
-    if (virAsprintf(&path, CPU_SYS_PATH "/cpu%u/topology/physical_package_id",
-                    cpu) < 0) {
-        virReportOOMError();
-        return -1;
-    }
-
-    pathfp = fopen(path, "r");
-    if (pathfp == NULL) {
-        virReportSystemError(errno, _("cannot open %s"), path);
-        VIR_FREE(path);
-        return -1;
-    }
-
-    if (fgets(socket_str, sizeof(socket_str), pathfp) == NULL) {
-        virReportSystemError(errno, _("cannot read from %s"), path);
-        goto cleanup;
-    }
-    if (virStrToLong_i(socket_str, &tmp, 10, &socket) < 0) {
-        nodeReportError(VIR_ERR_INTERNAL_ERROR,
-                        _("could not convert '%s' to an integer"),
-                        socket_str);
-        goto cleanup;
-    }
-
-cleanup:
-    fclose(pathfp);
-    VIR_FREE(path);
-
-    return socket;
+    return get_cpu_value(cpu, "topology/physical_package_id", false);
 }

 int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
@@ -153,6 +172,8 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
     unsigned long cur_threads;
     int socket;
     unsigned long long socket_mask = 0;
+    unsigned int remaining;
+    int online;

     nodeinfo->cpus = 0;
     nodeinfo->mhz = 0;
@@ -222,15 +243,25 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
     /* OK, we've parsed what we can out of /proc/cpuinfo.  Get the socket
      * and thread information from /sys
      */
+    remaining = nodeinfo->cpus;
     cpudir = opendir(CPU_SYS_PATH);
     if (cpudir == NULL) {
         virReportSystemError(errno, _("cannot opendir %s"), CPU_SYS_PATH);
         return -1;
     }
-    while ((cpudirent = readdir(cpudir))) {
+    while (errno = 0, remaining && (cpudirent = readdir(cpudir))) {
         if (sscanf(cpudirent->d_name, "cpu%u", &cpu) != 1)
             continue;

+        online = cpu_online(cpu);
+        if (online < 0) {
+            closedir(cpudir);
+            return -1;
+        }
+        if (!online)
+            continue;
+        remaining--;
+
         socket = parse_socket(cpu);
         if (socket < 0) {
             closedir(cpudir);
@@ -249,6 +280,12 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
         if (cur_threads > nodeinfo->threads)
             nodeinfo->threads = cur_threads;
     }
+    if (errno) {
+        virReportSystemError(errno,
+                             _("problem reading %s"), CPU_SYS_PATH);
+        closedir(cpudir);
+        return -1;
+    }

     closedir(cpudir);

-- 
1.7.1