[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH 1/2] 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>
---
 src/conf/numatune_conf.c | 24 +++++++++++++++
 src/conf/numatune_conf.h |  3 ++
 src/libvirt_private.syms |  1 +
 src/lxc/lxc_controller.c | 17 +++++++---
 src/qemu/qemu_command.c  |  2 +-
 src/qemu/qemu_process.c  |  9 +++++-
 src/util/virnuma.c       | 80 ++++++++++++++++--------------------------------
 src/util/virnuma.h       |  7 ++---
 tests/qemuxml2argvmock.c | 12 ++++++++
 9 files changed, 91 insertions(+), 64 deletions(-)

diff --git a/src/conf/numatune_conf.c b/src/conf/numatune_conf.c
index 6986739..ad928e0 100644
--- 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 15dc0d6..7ca7f97 100644
--- 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 8895ba1..7e38cc6 100644
--- 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 1e4b9bc..53a2c8d 100644
--- 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 13b54dd..a6e19b4 100644
--- 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 26d4948..002bd32 100644
--- 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 ee1c4af..20f1e56 100644
--- 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);
@@ -163,34 +158,6 @@ virNumaSetupMemoryPolicy(virDomainNumatunePtr numatune,
 }

 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)
 {
     return numa_available() != -1;
@@ -342,28 +309,16 @@ 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)
 {
     return false;
@@ -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 5bb37b8..09034a2 100644
--- 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 7218747..eccf4b0 100644
--- 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 */
-- 
2.1.3


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]