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 <chen.fan.f...@cn.fujitsu.com> --- 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 <linux/capability.h> #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 numatune);
bool virNumaIsAvailable(void);
int virNumaGetMaxNode(void);
bool virNumaNodeIsAvailable(int node);
diff --git
a/tests/qemuxml2argvdata/qemuxml2argv-numatune-static-nodeset-exceed-hostnode.xml
b/tests/qemuxml2argvdata/qemuxml2argv-numatune-static-nodeset-exceed-hostnode.xml
new file mode 100644
index 0000000..b7564fe
--- /dev/null
+++
b/tests/qemuxml2argvdata/qemuxml2argv-numatune-static-nodeset-exceed-hostnode.xml
@@ -0,0 +1,36 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>1048576</memory>
+ <currentMemory unit='KiB'>1048576</currentMemory>
+ <vcpu placement='static'>4</vcpu>
+ <numatune>
+ <memnode cellid='0' mode='strict' nodeset='0-8'/>
+ <memnode cellid='1' mode='strict' nodeset='0'/>
+ </numatune>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <cpu>
+ <numa>
+ <cell id='0' cpus='0-1' memory='524288'/>
+ <cell id='1' cpus='2-3' memory='524288'/>
+ </numa>
+ </cpu>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-system-x86_64</emulator>
+ <disk type='block' device='disk'>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='hda' bus='ide'/>
+ <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+ </disk>
+ <controller type='ide' index='0'/>
+ <memballoon model='virtio'/>
+ </devices>
+</domain>
+
Empty line at EOF. "make syntax-check" would find that for you ;)
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 <config.h>
#include "internal.h"
+#include "virnuma.h"
#include <time.h>
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;" ???
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 0e9fab9..bab6d0d 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1250,6 +1250,7 @@ mymain(void)
DO_TEST_FAILURE("numatune-memnode-no-memory", NONE);
DO_TEST("numatune-auto-nodeset-invalid", NONE);
+ DO_TEST_FAILURE("numatune-static-nodeset-exceed-hostnode",
QEMU_CAPS_OBJECT_MEMORY_RAM);
Very long line.
DO_TEST_PARSE_ERROR("numatune-memnode-nocpu", NONE);
DO_TEST_PARSE_ERROR("numatune-memnodes-problematic", NONE);
DO_TEST("numad", NONE);
--
1.9.3
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
