Re: [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 <chen.fan.fnst(a)cn.fujitsu.com&gt; --- 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)

+ 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-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> +

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..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);

DO_TEST_PARSE_ERROR("numatune-memnode-nocpu", NONE); DO_TEST_PARSE_ERROR("numatune-memnodes-problematic", NONE); DO_TEST("numad", NONE); -- 1.9.3