7:24 a.m.
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(a)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