[libvirt] [PATCH v5 0/3] add nodeset check in numatune
when setting elements memnode and nodeset in attribute numatune more than the host nodes in XML file, VM boot should fail. so add check for that. Chen Fan (3): bitmap: add virBitmapLastSetBit for finding the last bit position of bitmap numatune: add check for numatune nodeset range virnuma: use virNumaNodesetIsAvailable checking nodeset in virNumaSetupMemoryPolicy src/conf/numatune_conf.c | 28 ++++++++++++ src/conf/numatune_conf.h | 1 + src/libvirt_private.syms | 2 + src/qemu/qemu_command.c | 4 ++ src/util/virbitmap.c | 45 ++++++++++++++++++ src/util/virbitmap.h | 3 ++ src/util/virnuma.c | 53 +++++++++++++++++----- src/util/virnuma.h | 1 + ...rgv-numatune-static-nodeset-exceed-hostnode.xml | 35 ++++++++++++++ tests/qemuxml2argvmock.c | 9 ++++ tests/qemuxml2argvtest.c | 2 + tests/virbitmaptest.c | 13 +++++- 12 files changed, 184 insertions(+), 12 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-static-nodeset-exceed-hostnode.xml -- 1.9.3
Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com> --- src/libvirt_private.syms | 1 + src/util/virbitmap.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ src/util/virbitmap.h | 3 +++ tests/virbitmaptest.c | 13 ++++++++++++- 4 files changed, 61 insertions(+), 1 deletion(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d63a8f0..1e2bc0a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1011,6 +1011,7 @@ virBitmapFree; virBitmapGetBit; virBitmapIsAllClear; virBitmapIsAllSet; +virBitmapLastSetBit; virBitmapNew; virBitmapNewCopy; virBitmapNewData; diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index b6bd074..04a2388 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -651,6 +651,51 @@ virBitmapNextSetBit(virBitmapPtr bitmap, ssize_t pos) } /** + * virBitmapLastSetBit: + * @bitmap: the bitmap + * + * Search for the last set bit in bitmap @bitmap. + * + * Returns the position of the found bit, or -1 if no bit is set. + */ +ssize_t +virBitmapLastSetBit(virBitmapPtr bitmap) +{ + ssize_t i; + int unusedBits; + ssize_t sz; + unsigned long bits; + + unusedBits = bitmap->map_len * VIR_BITMAP_BITS_PER_UNIT - bitmap->max_bit; + + sz = bitmap->map_len - 1; + if (unusedBits > 0) { + bits = bitmap->map[sz] & (VIR_BITMAP_BIT(VIR_BITMAP_BITS_PER_UNIT - unusedBits) - 1); + if (bits != 0) + goto found; + + sz--; + } + + for (; sz >= 0; sz--) { + bits = bitmap->map[sz]; + if (bits != 0) + goto found; + } + + if (bits == 0) + return -1; + + found: + for (i = VIR_BITMAP_BITS_PER_UNIT - 1; i >= 0; i--) { + if (bits & 1UL << i) + return i + sz * VIR_BITMAP_BITS_PER_UNIT; + } + + return -1; +} + +/** * virBitmapNextClearBit: * @bitmap: the bitmap * @pos: the position after which to search for a clear bit diff --git a/src/util/virbitmap.h b/src/util/virbitmap.h index 4493cc9..565264c 100644 --- a/src/util/virbitmap.h +++ b/src/util/virbitmap.h @@ -105,6 +105,9 @@ bool virBitmapIsAllClear(virBitmapPtr bitmap) ssize_t virBitmapNextSetBit(virBitmapPtr bitmap, ssize_t pos) ATTRIBUTE_NONNULL(1); +ssize_t virBitmapLastSetBit(virBitmapPtr bitmap) + ATTRIBUTE_NONNULL(1); + ssize_t virBitmapNextClearBit(virBitmapPtr bitmap, ssize_t pos) ATTRIBUTE_NONNULL(1); diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c index ea832ad..ac5f298 100644 --- a/tests/virbitmaptest.c +++ b/tests/virbitmaptest.c @@ -171,7 +171,7 @@ test3(const void *data ATTRIBUTE_UNUSED) return ret; } -/* test for virBitmapNextSetBit, virBitmapNextClearBit */ +/* test for virBitmapNextSetBit, virBitmapLastSetBit, virBitmapNextClearBit */ static int test4(const void *data ATTRIBUTE_UNUSED) { @@ -200,6 +200,9 @@ test4(const void *data ATTRIBUTE_UNUSED) if (virBitmapNextSetBit(bitmap, -1) != -1) goto error; + if (virBitmapLastSetBit(bitmap) != -1) + goto error; + for (i = 0; i < size; i++) { if (virBitmapNextClearBit(bitmap, i - 1) != i) goto error; @@ -232,6 +235,11 @@ test4(const void *data ATTRIBUTE_UNUSED) if (virBitmapNextSetBit(bitmap, i) != -1) goto error; + j = sizeof(bitsPos)/sizeof(int) - 1; + + if (virBitmapLastSetBit(bitmap) != bitsPos[j]) + goto error; + j = 0; i = -1; @@ -255,6 +263,9 @@ test4(const void *data ATTRIBUTE_UNUSED) if (virBitmapNextSetBit(bitmap, i) != -1) goto error; + if (virBitmapLastSetBit(bitmap) != size - 1) + goto error; + if (virBitmapNextClearBit(bitmap, -1) != -1) goto error; -- 1.9.3
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.fnst@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 | 35 ++++++++++++++++++++ tests/qemuxml2argvmock.c | 9 +++++ tests/qemuxml2argvtest.c | 2 ++ 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..2540bd2 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 < 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) @@ -330,6 +357,17 @@ virNumaSetupMemoryPolicy(virDomainNumatunePtr numatune, 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) 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..84a560a --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-numatune-static-nodeset-exceed-hostnode.xml @@ -0,0 +1,35 @@ +<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> 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; +} diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 0e9fab9..902d03a 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1250,6 +1250,8 @@ 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); DO_TEST_PARSE_ERROR("numatune-memnode-nocpu", NONE); DO_TEST_PARSE_ERROR("numatune-memnodes-problematic", NONE); DO_TEST("numad", NONE); -- 1.9.3
Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com> --- src/util/virnuma.c | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/src/util/virnuma.c b/src/util/virnuma.c index 2540bd2..89435de 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -98,16 +98,13 @@ virNumaSetupMemoryPolicy(virDomainNumatunePtr numatune, int maxnode = 0; virBitmapPtr tmp_nodemask = NULL; + if (!virNumaNodesetIsAvailable(numatune)) + return -1; + tmp_nodemask = virDomainNumatuneGetNodeset(numatune, nodemask, -1); if (!tmp_nodemask) return 0; - if (numa_available() < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("Host kernel is not aware of NUMA.")); - return -1; - } - maxnode = numa_max_node(); maxnode = maxnode < NUMA_NUM_NODES ? maxnode : NUMA_NUM_NODES; @@ -347,12 +344,8 @@ int virNumaSetupMemoryPolicy(virDomainNumatunePtr numatune, virBitmapPtr nodemask ATTRIBUTE_UNUSED) { - if (virDomainNumatuneGetNodeset(numatune, NULL, -1)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("libvirt is compiled without NUMA tuning support")); - + if (!virNumaNodesetIsAvailable(numatune)) return -1; - } return 0; } -- 1.9.3
On Tue, Nov 04, 2014 at 10:44:38AM +0800, Chen Fan wrote:
when setting elements memnode and nodeset in attribute numatune more than the host nodes in XML file, VM boot should fail. so add check for that.
ACK && Pushed, thank you for the patches. Martin
Chen Fan (3): bitmap: add virBitmapLastSetBit for finding the last bit position of bitmap numatune: add check for numatune nodeset range virnuma: use virNumaNodesetIsAvailable checking nodeset in virNumaSetupMemoryPolicy
src/conf/numatune_conf.c | 28 ++++++++++++ src/conf/numatune_conf.h | 1 + src/libvirt_private.syms | 2 + src/qemu/qemu_command.c | 4 ++ src/util/virbitmap.c | 45 ++++++++++++++++++ src/util/virbitmap.h | 3 ++ src/util/virnuma.c | 53 +++++++++++++++++----- src/util/virnuma.h | 1 + ...rgv-numatune-static-nodeset-exceed-hostnode.xml | 35 ++++++++++++++ tests/qemuxml2argvmock.c | 9 ++++ tests/qemuxml2argvtest.c | 2 + tests/virbitmaptest.c | 13 +++++- 12 files changed, 184 insertions(+), 12 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-static-nodeset-exceed-hostnode.xml
-- 1.9.3
participants (2)
-
Chen Fan -
Martin Kletzander