Re: [PATCH] qemu: Validate memory hotplug in domainValidateCallback instead of cmd line generator

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.

Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; ---

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