[PATCH] qemu: Validate memory hotplug in domainValidateCallback instead of cmd line generator
Daniel Henrique Barboza
danielhb413 at gmail.com
Mon Aug 31 14:14:29 UTC 2020
On 8/31/20 10:44 AM, Michal Privoznik wrote:
> When editing a domain with hotplug enabled, I removed the only
> NUMA node it had and got no error. I got the error later though,
> when starting the domain. This is not as user friendly as it can
> be. Move the validation call out from command line generator and
> into domain validator (which is called prior to starting cmd line
> generation anyway).
>
> When doing this, I had to remove memory-hotplug-nonuma xml2xml
> test case because there is no way the test case can succeed,
> obviously.
I wonder why x86 doesn't just turn on auto_enable_numa in QEMU to
create an empty NUMA node for hotplug. If the user set everything else
to enable mem hotplug (ram_slots, maxram_size, etc) then might as well
go the distance ...
(ps: pSeries has auto_enable_numa, which makes this comment above completely
biased :P )
>
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
Reviewed-by: Daniel Henrique Barboza <danielhb413 at gmail.com>
> src/qemu/qemu_command.c | 3 --
> src/qemu/qemu_domain.c | 2 ++
> src/qemu/qemu_validate.c | 3 ++
> tests/qemuxml2argvtest.c | 2 +-
> .../memory-hotplug-nonuma.xml | 28 -------------------
> tests/qemuxml2xmltest.c | 14 ++++++----
> 6 files changed, 14 insertions(+), 38 deletions(-)
> delete mode 100644 tests/qemuxml2xmloutdata/memory-hotplug-nonuma.xml
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 6798febf8d..bd98b0a97c 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -7123,9 +7123,6 @@ qemuBuildMemCommandLine(virCommandPtr cmd,
> virQEMUCapsPtr qemuCaps,
> qemuDomainObjPrivatePtr priv)
> {
> - if (qemuDomainDefValidateMemoryHotplug(def, qemuCaps, NULL) < 0)
> - return -1;
> -
> virCommandAddArg(cmd, "-m");
>
> if (virDomainDefHasMemoryHotplug(def)) {
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 21f24fceed..aa0f5c1a2d 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -8468,6 +8468,8 @@ qemuCheckMemoryDimmConflict(const virDomainDef *def,
>
> return false;
> }
> +
> +
> static int
> qemuDomainDefValidateMemoryHotplugDevice(const virDomainMemoryDef *mem,
> const virDomainDef *def)
> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> index c4b86326ad..6f3fee5427 100644
> --- a/src/qemu/qemu_validate.c
> +++ b/src/qemu/qemu_validate.c
> @@ -916,6 +916,9 @@ qemuValidateDomainDef(const virDomainDef *def,
> }
> }
>
> + if (qemuDomainDefValidateMemoryHotplug(def, qemuCaps, NULL) < 0)
> + return -1;
> +
> if (qemuValidateDomainDefClockTimers(def, qemuCaps) < 0)
> return -1;
>
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index 01839cb88c..e93948e3fc 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -2931,7 +2931,7 @@ mymain(void)
> DO_TEST("cpu-host-passthrough-features", QEMU_CAPS_KVM);
>
> DO_TEST_FAILURE("memory-align-fail", NONE);
> - DO_TEST_FAILURE("memory-hotplug-nonuma", QEMU_CAPS_DEVICE_PC_DIMM);
> + DO_TEST_PARSE_ERROR("memory-hotplug-nonuma", QEMU_CAPS_DEVICE_PC_DIMM);
> DO_TEST("memory-hotplug", NONE);
> DO_TEST("memory-hotplug", QEMU_CAPS_DEVICE_PC_DIMM, QEMU_CAPS_NUMA);
> DO_TEST("memory-hotplug-dimm", QEMU_CAPS_DEVICE_PC_DIMM, QEMU_CAPS_NUMA,
> diff --git a/tests/qemuxml2xmloutdata/memory-hotplug-nonuma.xml b/tests/qemuxml2xmloutdata/memory-hotplug-nonuma.xml
> deleted file mode 100644
> index 7c277f01a3..0000000000
> --- a/tests/qemuxml2xmloutdata/memory-hotplug-nonuma.xml
> +++ /dev/null
> @@ -1,28 +0,0 @@
> -<domain type='qemu'>
> - <name>QEMUGuest1</name>
> - <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> - <maxMemory slots='9' unit='KiB'>1233456789</maxMemory>
> - <memory unit='KiB'>219136</memory>
> - <currentMemory unit='KiB'>219136</currentMemory>
> - <vcpu placement='static'>1</vcpu>
> - <os>
> - <type arch='i686' machine='pc'>hvm</type>
> - <boot dev='hd'/>
> - </os>
> - <clock offset='utc'/>
> - <on_poweroff>destroy</on_poweroff>
> - <on_reboot>restart</on_reboot>
> - <on_crash>destroy</on_crash>
> - <devices>
> - <emulator>/usr/bin/qemu-system-i386</emulator>
> - <controller type='usb' index='0'>
> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
> - </controller>
> - <controller type='pci' index='0' model='pci-root'/>
> - <input type='mouse' bus='ps2'/>
> - <input type='keyboard' bus='ps2'/>
> - <memballoon model='virtio'>
> - <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
> - </memballoon>
> - </devices>
> -</domain>
> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index a07e2b7553..6eb008c8d2 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -303,18 +303,21 @@ mymain(void)
>
> DO_TEST("pages-discard", NONE);
> DO_TEST("pages-discard-hugepages", QEMU_CAPS_OBJECT_MEMORY_FILE);
> - DO_TEST("pages-dimm-discard", NONE);
> + DO_TEST("pages-dimm-discard", QEMU_CAPS_DEVICE_PC_DIMM);
> DO_TEST("hugepages-default", QEMU_CAPS_OBJECT_MEMORY_FILE);
> DO_TEST("hugepages-default-2M", QEMU_CAPS_OBJECT_MEMORY_FILE);
> DO_TEST("hugepages-default-system-size", QEMU_CAPS_OBJECT_MEMORY_FILE);
> DO_TEST("hugepages-nodeset", QEMU_CAPS_OBJECT_MEMORY_FILE);
> DO_TEST("hugepages-numa-default-2M", QEMU_CAPS_OBJECT_MEMORY_FILE);
> - DO_TEST("hugepages-numa-default-dimm", QEMU_CAPS_OBJECT_MEMORY_FILE);
> + DO_TEST("hugepages-numa-default-dimm", QEMU_CAPS_DEVICE_PC_DIMM,
> + QEMU_CAPS_OBJECT_MEMORY_FILE);
> DO_TEST("hugepages-numa-nodeset", QEMU_CAPS_OBJECT_MEMORY_FILE);
> DO_TEST("hugepages-numa-nodeset-part", QEMU_CAPS_OBJECT_MEMORY_FILE);
> DO_TEST("hugepages-shared", QEMU_CAPS_OBJECT_MEMORY_FILE);
> - DO_TEST("hugepages-memaccess", QEMU_CAPS_OBJECT_MEMORY_FILE);
> - DO_TEST("hugepages-memaccess2", QEMU_CAPS_OBJECT_MEMORY_FILE);
> + DO_TEST("hugepages-memaccess", QEMU_CAPS_DEVICE_PC_DIMM,
> + QEMU_CAPS_OBJECT_MEMORY_FILE);
> + DO_TEST("hugepages-memaccess2", QEMU_CAPS_DEVICE_PC_DIMM,
> + QEMU_CAPS_OBJECT_MEMORY_FILE);
> DO_TEST("hugepages-nvdimm", QEMU_CAPS_DEVICE_NVDIMM,
> QEMU_CAPS_OBJECT_MEMORY_FILE);
> DO_TEST("nosharepages", NONE);
> @@ -1260,8 +1263,7 @@ mymain(void)
> DO_TEST_CAPS_ARCH_LATEST("aarch64-features-sve", "aarch64");
>
> DO_TEST("memory-hotplug", NONE);
> - DO_TEST("memory-hotplug-nonuma", NONE);
> - DO_TEST("memory-hotplug-dimm", NONE);
> + DO_TEST("memory-hotplug-dimm", QEMU_CAPS_DEVICE_PC_DIMM);
> DO_TEST("memory-hotplug-nvdimm", QEMU_CAPS_DEVICE_NVDIMM);
> DO_TEST("memory-hotplug-nvdimm-access", QEMU_CAPS_DEVICE_NVDIMM);
> DO_TEST("memory-hotplug-nvdimm-label", QEMU_CAPS_DEVICE_NVDIMM);
>
More information about the libvir-list
mailing list