numa: split util/ and conf/ and support non-contiguous nodesets

This is a reaction to Michal's fix [1] for non-NUMA systems that also
splits out conf/ out of util/ because libvirt_util shouldn't require
libvirt_conf if it is the other way around.  This particular use case
worked, but we're trying to avoid it as mentioned [2], many times.

The only functions from virnuma.c that needed numatune_conf were
virDomainNumatuneNodesetIsAvailable() and virNumaSetupMemoryPolicy().
The first one should be in numatune_conf as it works with
virDomainNumatune, the second one just needs nodeset and mode, both of
which can be passed without the need of numatune_conf.

Apart from fixing that, this patch also fixes recently added
code (between commits d2460f85^..5c8515620) that doesn't support
non-contiguous nodesets.  It uses new function
virNumaNodesetIsAvailable(), which doesn't need a stub as it doesn't use
any libnuma functions, to check if every specified nodeset is available.

[1] https://www.redhat.com/archives/libvir-list/2014-November/msg00118.html
[2] http://www.redhat.com/archives/libvir-list/2011-June/msg01040.html

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>

diff --git a/src/conf/numatune_conf.c b/src/conf/numatune_conf.c
index 698673950ee908dd08b163ae840abeaa1ca2b6d0..ad928e0ded889318cf6bc6c0e9fc88a5f1c243e0 100644 (file)
--- a/src/conf/numatune_conf.c
+++ b/src/conf/numatune_conf.c
@@ -26,6 +26,7 @@
 
 #include "domain_conf.h"
 #include "viralloc.h"
+#include "virnuma.h"
 #include "virstring.h"
 
 #define VIR_FROM_THIS VIR_FROM_DOMAIN
@@ -640,3 +641,26 @@ virDomainNumatuneSpecifiedMaxNode(virDomainNumatunePtr numatune)
 
     return ret;
 }
+
+bool
+virDomainNumatuneNodesetIsAvailable(virDomainNumatunePtr numatune,
+                                    virBitmapPtr auto_nodeset)
+{
+    size_t i = 0;
+    virBitmapPtr b = NULL;
+
+    if (!numatune)
+        return true;
+
+    b = virDomainNumatuneGetNodeset(numatune, auto_nodeset, -1);
+    if (!virNumaNodesetIsAvailable(b))
+        return false;
+
+    for (i = 0; i < numatune->nmem_nodes; i++) {
+        b = virDomainNumatuneGetNodeset(numatune, auto_nodeset, i);
+        if (!virNumaNodesetIsAvailable(b))
+            return false;
+    }
+
+    return true;
+}

diff --git a/src/conf/numatune_conf.h b/src/conf/numatune_conf.h
index 15dc0d6bd70382f823b1cbc2c5803b27a286bac2..7ca7f976f2f182dbb1029049f15190f5f42159bf 100644 (file)
--- a/src/conf/numatune_conf.h
+++ b/src/conf/numatune_conf.h
@@ -103,4 +103,7 @@ bool virDomainNumatuneHasPlacementAuto(virDomainNumatunePtr numatune);
 bool virDomainNumatuneHasPerNodeBinding(virDomainNumatunePtr numatune);
 
 int virDomainNumatuneSpecifiedMaxNode(virDomainNumatunePtr numatune);
+
+bool virDomainNumatuneNodesetIsAvailable(virDomainNumatunePtr numatune,
+                                         virBitmapPtr auto_nodeset);
 #endif /* __NUMATUNE_CONF_H__ */

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 8895ba1bbf62e541f7b0333b95ae0ec9a52bb0dc..7e38cc6ea89d58bb4db37018d052542b1b02db8f 100644 (file)
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -605,6 +605,7 @@ virDomainNumatuneHasPlacementAuto;
 virDomainNumatuneMaybeFormatNodeset;
 virDomainNumatuneMemModeTypeFromString;
 virDomainNumatuneMemModeTypeToString;
+virDomainNumatuneNodesetIsAvailable;
 virDomainNumatuneParseXML;
 virDomainNumatunePlacementTypeFromString;
 virDomainNumatunePlacementTypeToString;

diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index 1e4b9bc1702ab10ee5c6a927dced495e1bd09e8b..53a2c8d67f1599ba94bcb0cefa72980440a6ec30 100644 (file)
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -685,22 +685,29 @@ static int virLXCControllerGetNumadAdvice(virLXCControllerPtr ctrl,
  */
 static int virLXCControllerSetupResourceLimits(virLXCControllerPtr ctrl)
 {
-    virBitmapPtr nodemask = NULL;
+    virBitmapPtr auto_nodeset = NULL;
     int ret = -1;
+    virBitmapPtr nodeset = NULL;
+    virDomainNumatuneMemMode mode;
+
+    if (virLXCControllerGetNumadAdvice(ctrl, &auto_nodeset) < 0)
+        goto cleanup;
+
+    nodeset = virDomainNumatuneGetNodeset(ctrl->def->numatune, auto_nodeset, -1);
+    mode = virDomainNumatuneGetMode(ctrl->def->numatune, -1);
 
-    if (virLXCControllerGetNumadAdvice(ctrl, &nodemask) < 0 ||
-        virNumaSetupMemoryPolicy(ctrl->def->numatune, nodemask) < 0)
+    if (virNumaSetupMemoryPolicy(mode, nodeset) < 0)
         goto cleanup;
 
     if (virLXCControllerSetupCpuAffinity(ctrl) < 0)
         goto cleanup;
 
-    if (virLXCCgroupSetup(ctrl->def, ctrl->cgroup, nodemask) < 0)
+    if (virLXCCgroupSetup(ctrl->def, ctrl->cgroup, nodeset) < 0)
         goto cleanup;
 
     ret = 0;
  cleanup:
-    virBitmapFree(nodemask);
+    virBitmapFree(auto_nodeset);
     return ret;
 }
 

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 90d404996e2872bfd8cef6e00597f1afef419572..3bed0ec3cc0c16033d2146072bf6a7d0a30c6432 100644 (file)
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6665,7 +6665,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
         goto cleanup;
     }
 
-    if (!virNumaNodesetIsAvailable(def->numatune))
+    if (!virDomainNumatuneNodesetIsAvailable(def->numatune, nodeset))
         goto cleanup;
 
     for (i = 0; i < def->mem.nhugepages; i++) {

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 26d494825a78f20a256769bf5a33cb67fc44d995..002bd3239de63fdcdaa5cb7c912f7de8bb6e7561 100644 (file)
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2924,6 +2924,9 @@ static int qemuProcessHook(void *data)
     struct qemuProcessHookData *h = data;
     int ret = -1;
     int fd;
+    virBitmapPtr nodeset = NULL;
+    virDomainNumatuneMemMode mode;
+
     /* This method cannot use any mutexes, which are not
      * protected across fork()
      */
@@ -2953,7 +2956,11 @@ static int qemuProcessHook(void *data)
     if (virSecurityManagerClearSocketLabel(h->driver->securityManager, h->vm->def) < 0)
         goto cleanup;
 
-    if (virNumaSetupMemoryPolicy(h->vm->def->numatune, h->nodemask) < 0)
+    mode = virDomainNumatuneGetMode(h->vm->def->numatune, -1);
+    nodeset = virDomainNumatuneGetNodeset(h->vm->def->numatune,
+                                          h->nodemask, -1);
+
+    if (virNumaSetupMemoryPolicy(mode, nodeset) < 0)
         goto cleanup;
 
     ret = 0;

diff --git a/src/util/virnuma.c b/src/util/virnuma.c
index ee1c4aff41b6f794ef27dcc796b59c17cd1bbebc..20f1e564b2b47de9a4332a88dbcaebb8ff131814 100644 (file)
--- a/src/util/virnuma.c
+++ b/src/util/virnuma.c
@@ -87,8 +87,8 @@ virNumaGetAutoPlacementAdvice(unsigned short vcpus ATTRIBUTE_UNUSED,
 
 #if WITH_NUMACTL
 int
-virNumaSetupMemoryPolicy(virDomainNumatunePtr numatune,
-                         virBitmapPtr nodemask)
+virNumaSetupMemoryPolicy(virDomainNumatuneMemMode mode,
+                         virBitmapPtr nodeset)
 {
     nodemask_t mask;
     int node = -1;
@@ -96,22 +96,17 @@ virNumaSetupMemoryPolicy(virDomainNumatunePtr numatune,
     int bit = 0;
     size_t i;
     int maxnode = 0;
-    virBitmapPtr tmp_nodemask = NULL;
 
-    if (!virNumaNodesetIsAvailable(numatune))
+    if (!virNumaNodesetIsAvailable(nodeset))
         return -1;
 
-    tmp_nodemask = virDomainNumatuneGetNodeset(numatune, nodemask, -1);
-    if (!tmp_nodemask)
-        return 0;
-
     maxnode = numa_max_node();
     maxnode = maxnode < NUMA_NUM_NODES ? maxnode : NUMA_NUM_NODES;
 
     /* Convert nodemask to NUMA bitmask. */
     nodemask_zero(&mask);
     bit = -1;
-    while ((bit = virBitmapNextSetBit(tmp_nodemask, bit)) >= 0) {
+    while ((bit = virBitmapNextSetBit(nodeset, bit)) >= 0) {
         if (bit > maxnode) {
             virReportError(VIR_ERR_INTERNAL_ERROR,
                            _("NUMA node %d is out of range"), bit);
@@ -120,7 +115,7 @@ virNumaSetupMemoryPolicy(virDomainNumatunePtr numatune,
         nodemask_set(&mask, bit);
     }
 
-    switch (virDomainNumatuneGetMode(numatune, -1)) {
+    switch (mode) {
     case VIR_DOMAIN_NUMATUNE_MEM_STRICT:
         numa_set_bind_policy(1);
         numa_set_membind(&mask);
@@ -162,34 +157,6 @@ virNumaSetupMemoryPolicy(virDomainNumatunePtr numatune,
     return ret;
 }
 
-bool
-virNumaNodesetIsAvailable(virDomainNumatunePtr numatune)
-{
-    int maxnode;
-    int bit;
-
-    if (!numatune)
-        return true;
-
-    bit = virDomainNumatuneSpecifiedMaxNode(numatune);
-    if (bit < 0)
-        return true;
-
-    if ((maxnode = virNumaGetMaxNode()) < 0)
-        return false;
-
-    maxnode = maxnode < NUMA_NUM_NODES ? maxnode : NUMA_NUM_NODES;
-    if (bit > maxnode)
-        goto error;
-
-    return true;
-
- error:
-    virReportError(VIR_ERR_INTERNAL_ERROR,
-                   _("NUMA node %d is out of range"), bit);
-    return false;
-}
-
 bool
 virNumaIsAvailable(void)
 {
@@ -342,27 +309,15 @@ virNumaGetNodeCPUs(int node,
 #else /* !WITH_NUMACTL */
 
 int
-virNumaSetupMemoryPolicy(virDomainNumatunePtr numatune,
-                         virBitmapPtr nodemask ATTRIBUTE_UNUSED)
+virNumaSetupMemoryPolicy(virDomainNumatuneMemMode mode ATTRIBUTE_UNUSED,
+                         virBitmapPtr nodeset)
 {
-    if (!virNumaNodesetIsAvailable(numatune))
+    if (!virNumaNodesetIsAvailable(nodeset))
         return -1;
 
     return 0;
 }
 
-bool
-virNumaNodesetIsAvailable(virDomainNumatunePtr numatune)
-{
-    if (virDomainNumatuneSpecifiedMaxNode(numatune) >= 0) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("libvirt is compiled without NUMA tuning support"));
-        return false;
-    }
-
-    return true;
-}
-
 bool
 virNumaIsAvailable(void)
 {
@@ -1006,3 +961,22 @@ virNumaSetPagePoolSize(int node ATTRIBUTE_UNUSED,
     return -1;
 }
 #endif /* #ifdef __linux__ */
+
+bool
+virNumaNodesetIsAvailable(virBitmapPtr nodeset)
+{
+    ssize_t bit = -1;
+
+    if (!nodeset)
+        return true;
+
+    while ((bit = virBitmapNextSetBit(nodeset, bit)) >= 0) {
+        if (virNumaNodeIsAvailable(bit))
+            continue;
+
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("NUMA node %zd is unavailable"), bit);
+        return false;
+    }
+    return true;
+}

diff --git a/src/util/virnuma.h b/src/util/virnuma.h
index 5bb37b81d27578b159fc7dfa03dc13237911e49d..09034a274b9bd6e71309d74814139042124e7962 100644 (file)
--- a/src/util/virnuma.h
+++ b/src/util/virnuma.h
@@ -23,7 +23,6 @@
 # define __VIR_NUMA_H__
 
 # include "internal.h"
-# include "numatune_conf.h"
 # include "virbitmap.h"
 # include "virutil.h"
 
@@ -31,10 +30,10 @@
 char *virNumaGetAutoPlacementAdvice(unsigned short vcups,
                                     unsigned long long balloon);
 
-int virNumaSetupMemoryPolicy(virDomainNumatunePtr numatune,
-                             virBitmapPtr nodemask);
+int virNumaSetupMemoryPolicy(virDomainNumatuneMemMode mode,
+                             virBitmapPtr nodeset);
 
-bool virNumaNodesetIsAvailable(virDomainNumatunePtr numatune);
+bool virNumaNodesetIsAvailable(virBitmapPtr nodeset);
 bool virNumaIsAvailable(void);
 int virNumaGetMaxNode(void);
 bool virNumaNodeIsAvailable(int node);

diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c
index 7218747f75127cee589aeb7fd6a1948ac4fa9bc3..eccf4b0966eff28dcfa8d58e963524d69434d8a1 100644 (file)
--- a/tests/qemuxml2argvmock.c
+++ b/tests/qemuxml2argvmock.c
@@ -39,3 +39,15 @@ virNumaGetMaxNode(void)
 
    return maxnodesNum;
 }
+
+#if WITH_NUMACTL && HAVE_NUMA_BITMASK_ISBITSET
+/*
+ * In case libvirt is compiled with full NUMA support, we need to mock
+ * this function in order to fake what numa nodes are available.
+ */
+bool
+virNumaNodeIsAvailable(int node)
+{
+    return node >= 0 && node <= virNumaGetMaxNode();
+}
+#endif /* WITH_NUMACTL && HAVE_NUMA_BITMASK_ISBITSET */