From 1a4abb61662ff2090ffb1c1f44cc626101742764 Mon Sep 17 00:00:00 2001

Message-Id: <1a4abb61662ff2090ffb1c1f44cc626101742764@dist-git>

From: Andrea Bolognani <abologna@redhat.com>

Date: Wed, 5 Aug 2015 18:18:28 +0200

Subject: [PATCH] nodeinfo: Phase out cpu_set_t usage

Swap out all instances of cpu_set_t and replace them with virBitmap,

which some of the code was already using anyway.

The changes are pretty mechanical, with one notable exception: an

assumption has been added on the max value we can run into while

reading either socket_it or core_id.

While this specific assumption was not in place before, we were

using cpu_set_t improperly by not making sure not to set any bit

past CPU_SETSIZE or explicitly allocating bigger bitmaps; in fact

the default size of a cpu_set_t, 1024, is way too low to run our

testsuite, which includes core_id values in the 2000s.

(cherry picked from commit b7b506475ca176276bfaa5dbcdba5b16744b6d56)

Bug: https://bugzilla.redhat.com/show_bug.cgi?id=1213713

Signed-off-by: Andrea Bolognani <abologna@redhat.com>

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>

---

 src/nodeinfo.c | 65 ++++++++++++++++++++++++++++++++++------------------------

 1 file changed, 38 insertions(+), 27 deletions(-)

diff --git a/src/nodeinfo.c b/src/nodeinfo.c

index 3550e24..7d0e6b0 100644

--- a/src/nodeinfo.c

+++ b/src/nodeinfo.c

@@ -30,7 +30,6 @@

 #include <errno.h>

 #include <dirent.h>

 #include <sys/utsname.h>

-#include <sched.h>

 #include "conf/domain_conf.h"

 #if defined(__FreeBSD__) || defined(__APPLE__)

@@ -388,19 +387,6 @@ virNodeParseSocket(const char *dir,

     return ret;

 }

-# ifndef CPU_COUNT

-static int

-CPU_COUNT(cpu_set_t *set)

-{

-    size_t i, count = 0;

-

-    for (i = 0; i < CPU_SETSIZE; i++)

-        if (CPU_ISSET(i, set))

-            count++;

-    return count;

-}

-# endif /* !CPU_COUNT */

-

 /* parses a node entry, returning number of processors in the node and

  * filling arguments */

 static int

@@ -415,15 +401,18 @@ virNodeParseNode(const char *sysfs_prefix,

                  int *threads,

                  int *offline)

 {

+    /* Biggest value we can expect to be used as either socket id

+     * or core id. Bitmaps will need to be sized accordingly */

+    const int ID_MAX = 4095;

     int ret = -1;

     int processors = 0;

     DIR *cpudir = NULL;

     struct dirent *cpudirent = NULL;

     virBitmapPtr present_cpumap = NULL;

+    virBitmapPtr sockets_map = NULL;

+    virBitmapPtr *cores_maps = NULL;

     int sock_max = 0;

-    cpu_set_t sock_map;

     int sock;

-    cpu_set_t *core_maps = NULL;

     int core;

     size_t i;

     int siblings;

@@ -445,7 +434,9 @@ virNodeParseNode(const char *sysfs_prefix,

         goto cleanup;

     /* enumerate sockets in the node */

-    CPU_ZERO(&sock_map);

+    if (!(sockets_map = virBitmapNew(ID_MAX + 1)))

+        goto cleanup;

+

     while ((direrr = virDirRead(cpudir, &cpudirent, node)) > 0) {

         if (sscanf(cpudirent->d_name, "cpu%u", &cpu) != 1)

             continue;

@@ -462,7 +453,15 @@ virNodeParseNode(const char *sysfs_prefix,

         /* Parse socket */

         if ((sock = virNodeParseSocket(node, arch, cpu)) < 0)

             goto cleanup;

-        CPU_SET(sock, &sock_map);

+        if (sock > ID_MAX) {

+            virReportError(VIR_ERR_INTERNAL_ERROR,

+                           _("Socket %d can't be handled (max socket is %d)"),

+                           sock, ID_MAX);

+            goto cleanup;

+        }

+

+        if (virBitmapSetBit(sockets_map, sock) < 0)

+            goto cleanup;

         if (sock > sock_max)

             sock_max = sock;

@@ -473,12 +472,13 @@ virNodeParseNode(const char *sysfs_prefix,

     sock_max++;

-    /* allocate cpu maps for each socket */

-    if (VIR_ALLOC_N(core_maps, sock_max) < 0)

+    /* allocate cores maps for each socket */

+    if (VIR_ALLOC_N(cores_maps, sock_max) < 0)

         goto cleanup;

     for (i = 0; i < sock_max; i++)

-        CPU_ZERO(&core_maps[i]);

+        if (!(cores_maps[i] = virBitmapNew(ID_MAX + 1)))

+            goto cleanup;

     /* iterate over all CPU's in the node */

     rewinddir(cpudir);

@@ -502,7 +502,7 @@ virNodeParseNode(const char *sysfs_prefix,

         /* Parse socket */

         if ((sock = virNodeParseSocket(node, arch, cpu)) < 0)

             goto cleanup;

-        if (!CPU_ISSET(sock, &sock_map)) {

+        if (!virBitmapIsBitSet(sockets_map, sock)) {

             virReportError(VIR_ERR_INTERNAL_ERROR, "%s",

                            _("CPU socket topology has changed"));

             goto cleanup;

@@ -515,8 +515,15 @@ virNodeParseNode(const char *sysfs_prefix,

         } else {

             core = virNodeGetCpuValue(node, cpu, "topology/core_id", 0);

         }

+        if (core > ID_MAX) {

+            virReportError(VIR_ERR_INTERNAL_ERROR,

+                           _("Core %d can't be handled (max core is %d)"),

+                           core, ID_MAX);

+            goto cleanup;

+        }

-        CPU_SET(core, &core_maps[sock]);

+        if (virBitmapSetBit(cores_maps[sock], core) < 0)

+            goto cleanup;

         if (!(siblings = virNodeCountThreadSiblings(node, cpu)))

             goto cleanup;

@@ -529,13 +536,13 @@ virNodeParseNode(const char *sysfs_prefix,

         goto cleanup;

     /* finalize the returned data */

-    *sockets = CPU_COUNT(&sock_map);

+    *sockets = virBitmapCountBits(sockets_map);

     for (i = 0; i < sock_max; i++) {

-        if (!CPU_ISSET(i, &sock_map))

+        if (!virBitmapIsBitSet(sockets_map, i))

             continue;

-        core = CPU_COUNT(&core_maps[i]);

+        core = virBitmapCountBits(cores_maps[i]);

         if (core > *cores)

             *cores = core;

     }

@@ -548,7 +555,11 @@ virNodeParseNode(const char *sysfs_prefix,

         virReportSystemError(errno, _("problem closing %s"), node);

         ret = -1;

     }

-    VIR_FREE(core_maps);

+    if (cores_maps)

+        for (i = 0; i < sock_max; i++)

+            virBitmapFree(cores_maps[i]);

+    VIR_FREE(cores_maps);

+    virBitmapFree(sockets_map);

     virBitmapFree(present_cpumap);

     return ret;

-- 

2.5.0