Re: [libvirt] [PATCH v4 2/3] numatune: add check for numatune nodeset range
On Tue, Nov 04, 2014 at 01:57:52AM +, Chen, Fan wrote:
On Mon, 2014-11-03 at 14:18 +0100, Martin Kletzander wrote:
On Thu, Oct 30, 2014 at 01:44:18PM +0800, Chen Fan wrote:
>diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c
>index bff3b0f..7218747 100644
>--- a/tests/qemuxml2argvmock.c
>+++ b/tests/qemuxml2argvmock.c
>@@ -21,6 +21,7 @@
> #include
>
> #include "internal.h"
>+#include "virnuma.h"
> #include
>
> time_t time(time_t *t)
>@@ -30,3 +31,11 @@ time_t time(time_t *t)
> *t = ret;
> return ret;
> }
>+
>+int
>+virNumaGetMaxNode(void)
>+{
>+ const int maxnodesNum = 7;
>+
>+ return maxnodesNum;
>+}
Why not just "return 7;" ???
I just think magic number may be not proper.
Probably a matter of taste, I'd use a comment in that case, but proper
compiler should optimize it even without that const, so no problem
here ;)
Martin
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 2/3] numatune: add check for numatune nodeset range
On Mon, 2014-11-03 at 14:18 +0100, Martin Kletzander wrote:
> On Thu, Oct 30, 2014 at 01:44:18PM +0800, Chen Fan wrote:
> >There was no check for 'nodeset' attribute in numatune-related
> >elements. This patch adds validation that any nodeset specified does
> >not exceed maximum host node.
> >
> >Signed-off-by: Chen Fan
> >---
> > src/conf/numatune_conf.c | 28
> > src/conf/numatune_conf.h | 1 +
> > src/libvirt_private.syms | 1 +
> > src/qemu/qemu_command.c| 4 +++
> > src/util/virnuma.c | 38
> > ++
> > src/util/virnuma.h | 1 +
> > ...rgv-numatune-static-nodeset-exceed-hostnode.xml | 36
> > tests/qemuxml2argvmock.c | 9 +
> > tests/qemuxml2argvtest.c | 1 +
> > 9 files changed, 119 insertions(+)
> > create mode 100644
> > tests/qemuxml2argvdata/qemuxml2argv-numatune-static-nodeset-exceed-hostnode.xml
> >
> >diff --git a/src/conf/numatune_conf.c b/src/conf/numatune_conf.c
> >index 21d9a64..6986739 100644
> >--- a/src/conf/numatune_conf.c
> >+++ b/src/conf/numatune_conf.c
> >@@ -612,3 +612,31 @@ virDomainNumatuneHasPerNodeBinding(virDomainNumatunePtr
> >numatune)
> >
> > return false;
> > }
> >+
> >+int
> >+virDomainNumatuneSpecifiedMaxNode(virDomainNumatunePtr numatune)
> >+{
> >+int ret = -1;
> >+virBitmapPtr nodemask = NULL;
> >+size_t i;
> >+int bit;
> >+
> >+if (!numatune)
> >+return ret;
> >+
> >+nodemask = virDomainNumatuneGetNodeset(numatune, NULL, -1);
> >+if (nodemask)
> >+ret = virBitmapLastSetBit(nodemask);
> >+
> >+for (i = 0; i < numatune->nmem_nodes; i++) {
> >+nodemask = numatune->mem_nodes[i].nodeset;
> >+if (!nodemask)
> >+continue;
> >+
> >+bit = virBitmapLastSetBit(nodemask);
> >+if (bit > ret)
> >+ret = bit;
> >+}
> >+
> >+return ret;
> >+}
> >diff --git a/src/conf/numatune_conf.h b/src/conf/numatune_conf.h
> >index 5254629..15dc0d6 100644
> >--- a/src/conf/numatune_conf.h
> >+++ b/src/conf/numatune_conf.h
> >@@ -102,4 +102,5 @@ bool
> >virDomainNumatuneHasPlacementAuto(virDomainNumatunePtr numatune);
> >
> > bool virDomainNumatuneHasPerNodeBinding(virDomainNumatunePtr numatune);
> >
> >+int virDomainNumatuneSpecifiedMaxNode(virDomainNumatunePtr numatune);
> > #endif /* __NUMATUNE_CONF_H__ */
> >diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> >index 1e2bc0a..4a30ad7 100644
> >--- a/src/libvirt_private.syms
> >+++ b/src/libvirt_private.syms
> >@@ -1729,6 +1729,7 @@ virNumaGetPageInfo;
> > virNumaGetPages;
> > virNumaIsAvailable;
> > virNumaNodeIsAvailable;
> >+virNumaNodesetIsAvailable;
> > virNumaSetPagePoolSize;
> > virNumaSetupMemoryPolicy;
> >
> >diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> >index 2e5af4f..d2c5f0c 100644
> >--- a/src/qemu/qemu_command.c
> >+++ b/src/qemu/qemu_command.c
> >@@ -53,6 +53,7 @@
> > #include "virstoragefile.h"
> > #include "virtpm.h"
> > #include "virscsi.h"
> >+#include "virnuma.h"
> > #if defined(__linux__)
> > # include
> > #endif
> >@@ -6663,6 +6664,9 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
> > goto cleanup;
> > }
> >
> >+if (!virNumaNodesetIsAvailable(def->numatune))
> >+goto cleanup;
> >+
> > for (i = 0; i < def->mem.nhugepages; i++) {
> > ssize_t next_bit, pos = 0;
> >
> >diff --git a/src/util/virnuma.c b/src/util/virnuma.c
> >index 690615f..4188ef5 100644
> >--- a/src/util/virnuma.c
> >+++ b/src/util/virnuma.c
> >@@ -165,6 +165,33 @@ virNumaSetupMemoryPolicy(virDomainNumatunePtr numatune,
> > return ret;
> > }
> >
> >+bool
> >+virNumaNodesetIsAvailable(virDomainNumatunePtr numatune)
> >+{
> >+int maxnode;
> >+int bit;
> >+
> >+if (!numatune)
> >+return true;
> >+
> >+bit = virDomainNumatuneSpecifiedMaxNode(numatune);
> >+if (bit == -1)
>
> (bit < 0) would go with the rest of the code, the functions can be
> then modified to report various kinds of errors more easily.
>
> >+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)
> >@@ -330,6 +357,17 @@ virNumaSetupMemoryPolicy(virDomainNumatunePtr numatune,
> > return 0;
> > }
> >
> >+bool
> >+virNumaNodesetIsAvailable(virDomainNumatunePtr numatune)
> >+{
> >+if (virDomainNumatuneSpecifiedMaxNode(numatune) != -1) {
>
> similarly " >= 0" here.
>
> >+virReportError(VIR_ERR_INT
Re: [libvirt] [PATCH v4 2/3] numatune: add check for numatune nodeset range
On Thu, Oct 30, 2014 at 01:44:18PM +0800, Chen Fan wrote:
There was no check for 'nodeset' attribute in numatune-related
elements. This patch adds validation that any nodeset specified does
not exceed maximum host node.
Signed-off-by: Chen Fan
---
src/conf/numatune_conf.c | 28
src/conf/numatune_conf.h | 1 +
src/libvirt_private.syms | 1 +
src/qemu/qemu_command.c| 4 +++
src/util/virnuma.c | 38 ++
src/util/virnuma.h | 1 +
...rgv-numatune-static-nodeset-exceed-hostnode.xml | 36
tests/qemuxml2argvmock.c | 9 +
tests/qemuxml2argvtest.c | 1 +
9 files changed, 119 insertions(+)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-numatune-static-nodeset-exceed-hostnode.xml
diff --git a/src/conf/numatune_conf.c b/src/conf/numatune_conf.c
index 21d9a64..6986739 100644
--- a/src/conf/numatune_conf.c
+++ b/src/conf/numatune_conf.c
@@ -612,3 +612,31 @@ virDomainNumatuneHasPerNodeBinding(virDomainNumatunePtr
numatune)
return false;
}
+
+int
+virDomainNumatuneSpecifiedMaxNode(virDomainNumatunePtr numatune)
+{
+int ret = -1;
+virBitmapPtr nodemask = NULL;
+size_t i;
+int bit;
+
+if (!numatune)
+return ret;
+
+nodemask = virDomainNumatuneGetNodeset(numatune, NULL, -1);
+if (nodemask)
+ret = virBitmapLastSetBit(nodemask);
+
+for (i = 0; i < numatune->nmem_nodes; i++) {
+nodemask = numatune->mem_nodes[i].nodeset;
+if (!nodemask)
+continue;
+
+bit = virBitmapLastSetBit(nodemask);
+if (bit > ret)
+ret = bit;
+}
+
+return ret;
+}
diff --git a/src/conf/numatune_conf.h b/src/conf/numatune_conf.h
index 5254629..15dc0d6 100644
--- a/src/conf/numatune_conf.h
+++ b/src/conf/numatune_conf.h
@@ -102,4 +102,5 @@ bool virDomainNumatuneHasPlacementAuto(virDomainNumatunePtr
numatune);
bool virDomainNumatuneHasPerNodeBinding(virDomainNumatunePtr numatune);
+int virDomainNumatuneSpecifiedMaxNode(virDomainNumatunePtr numatune);
#endif /* __NUMATUNE_CONF_H__ */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 1e2bc0a..4a30ad7 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1729,6 +1729,7 @@ virNumaGetPageInfo;
virNumaGetPages;
virNumaIsAvailable;
virNumaNodeIsAvailable;
+virNumaNodesetIsAvailable;
virNumaSetPagePoolSize;
virNumaSetupMemoryPolicy;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 2e5af4f..d2c5f0c 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -53,6 +53,7 @@
#include "virstoragefile.h"
#include "virtpm.h"
#include "virscsi.h"
+#include "virnuma.h"
#if defined(__linux__)
# include
#endif
@@ -6663,6 +6664,9 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
goto cleanup;
}
+if (!virNumaNodesetIsAvailable(def->numatune))
+goto cleanup;
+
for (i = 0; i < def->mem.nhugepages; i++) {
ssize_t next_bit, pos = 0;
diff --git a/src/util/virnuma.c b/src/util/virnuma.c
index 690615f..4188ef5 100644
--- a/src/util/virnuma.c
+++ b/src/util/virnuma.c
@@ -165,6 +165,33 @@ virNumaSetupMemoryPolicy(virDomainNumatunePtr numatune,
return ret;
}
+bool
+virNumaNodesetIsAvailable(virDomainNumatunePtr numatune)
+{
+int maxnode;
+int bit;
+
+if (!numatune)
+return true;
+
+bit = virDomainNumatuneSpecifiedMaxNode(numatune);
+if (bit == -1)
(bit < 0) would go with the rest of the code, the functions can be
then modified to report various kinds of errors more easily.
+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)
@@ -330,6 +357,17 @@ virNumaSetupMemoryPolicy(virDomainNumatunePtr numatune,
return 0;
}
+bool
+virNumaNodesetIsAvailable(virDomainNumatunePtr numatune)
+{
+if (virDomainNumatuneSpecifiedMaxNode(numatune) != -1) {
similarly " >= 0" here.
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("libvirt is compiled without NUMA tuning support"));
+return false;
+}
+
+return true;
+}
bool
virNumaIsAvailable(void)
diff --git a/src/util/virnuma.h b/src/util/virnuma.h
index 04b6b8c..5bb37b8 100644
--- a/src/util/virnuma.h
+++ b/src/util/virnuma.h
@@ -34,6 +34,7 @@ char *virNumaGetAutoPlacementAdvice(unsigned short vcups,
int virNumaSetupMemoryPolicy(virDomainNumatunePtr numatune,
virBitmapPtr nodemask);
+bool virNumaNodesetIsAvailable(virDomainNumatunePtr
[libvirt] [PATCH v4 2/3] numatune: add check for numatune nodeset range
There was no check for 'nodeset' attribute in numatune-related
elements. This patch adds validation that any nodeset specified does
not exceed maximum host node.
Signed-off-by: Chen Fan
---
src/conf/numatune_conf.c | 28
src/conf/numatune_conf.h | 1 +
src/libvirt_private.syms | 1 +
src/qemu/qemu_command.c| 4 +++
src/util/virnuma.c | 38 ++
src/util/virnuma.h | 1 +
...rgv-numatune-static-nodeset-exceed-hostnode.xml | 36
tests/qemuxml2argvmock.c | 9 +
tests/qemuxml2argvtest.c | 1 +
9 files changed, 119 insertions(+)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-numatune-static-nodeset-exceed-hostnode.xml
diff --git a/src/conf/numatune_conf.c b/src/conf/numatune_conf.c
index 21d9a64..6986739 100644
--- a/src/conf/numatune_conf.c
+++ b/src/conf/numatune_conf.c
@@ -612,3 +612,31 @@ virDomainNumatuneHasPerNodeBinding(virDomainNumatunePtr
numatune)
return false;
}
+
+int
+virDomainNumatuneSpecifiedMaxNode(virDomainNumatunePtr numatune)
+{
+int ret = -1;
+virBitmapPtr nodemask = NULL;
+size_t i;
+int bit;
+
+if (!numatune)
+return ret;
+
+nodemask = virDomainNumatuneGetNodeset(numatune, NULL, -1);
+if (nodemask)
+ret = virBitmapLastSetBit(nodemask);
+
+for (i = 0; i < numatune->nmem_nodes; i++) {
+nodemask = numatune->mem_nodes[i].nodeset;
+if (!nodemask)
+continue;
+
+bit = virBitmapLastSetBit(nodemask);
+if (bit > ret)
+ret = bit;
+}
+
+return ret;
+}
diff --git a/src/conf/numatune_conf.h b/src/conf/numatune_conf.h
index 5254629..15dc0d6 100644
--- a/src/conf/numatune_conf.h
+++ b/src/conf/numatune_conf.h
@@ -102,4 +102,5 @@ bool virDomainNumatuneHasPlacementAuto(virDomainNumatunePtr
numatune);
bool virDomainNumatuneHasPerNodeBinding(virDomainNumatunePtr numatune);
+int virDomainNumatuneSpecifiedMaxNode(virDomainNumatunePtr numatune);
#endif /* __NUMATUNE_CONF_H__ */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 1e2bc0a..4a30ad7 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1729,6 +1729,7 @@ virNumaGetPageInfo;
virNumaGetPages;
virNumaIsAvailable;
virNumaNodeIsAvailable;
+virNumaNodesetIsAvailable;
virNumaSetPagePoolSize;
virNumaSetupMemoryPolicy;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 2e5af4f..d2c5f0c 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -53,6 +53,7 @@
#include "virstoragefile.h"
#include "virtpm.h"
#include "virscsi.h"
+#include "virnuma.h"
#if defined(__linux__)
# include
#endif
@@ -6663,6 +6664,9 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
goto cleanup;
}
+if (!virNumaNodesetIsAvailable(def->numatune))
+goto cleanup;
+
for (i = 0; i < def->mem.nhugepages; i++) {
ssize_t next_bit, pos = 0;
diff --git a/src/util/virnuma.c b/src/util/virnuma.c
index 690615f..4188ef5 100644
--- a/src/util/virnuma.c
+++ b/src/util/virnuma.c
@@ -165,6 +165,33 @@ virNumaSetupMemoryPolicy(virDomainNumatunePtr numatune,
return ret;
}
+bool
+virNumaNodesetIsAvailable(virDomainNumatunePtr numatune)
+{
+int maxnode;
+int bit;
+
+if (!numatune)
+return true;
+
+bit = virDomainNumatuneSpecifiedMaxNode(numatune);
+if (bit == -1)
+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)
@@ -330,6 +357,17 @@ virNumaSetupMemoryPolicy(virDomainNumatunePtr numatune,
return 0;
}
+bool
+virNumaNodesetIsAvailable(virDomainNumatunePtr numatune)
+{
+if (virDomainNumatuneSpecifiedMaxNode(numatune) != -1) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("libvirt is compiled without NUMA tuning support"));
+return false;
+}
+
+return true;
+}
bool
virNumaIsAvailable(void)
diff --git a/src/util/virnuma.h b/src/util/virnuma.h
index 04b6b8c..5bb37b8 100644
--- a/src/util/virnuma.h
+++ b/src/util/virnuma.h
@@ -34,6 +34,7 @@ char *virNumaGetAutoPlacementAdvice(unsigned short vcups,
int virNumaSetupMemoryPolicy(virDomainNumatunePtr numatune,
virBitmapPtr nodemask);
+bool virNumaNodesetIsAvailable(virDomainNumatunePtr numatune);
bool virNumaIsAvailable(void);
int virNumaGetMaxNode(void);
bool virNumaNodeIsAvailable(int node);
diff --git
a/tests/qemuxml2argvdata/qemuxml2argv-numat
