From f7baee531ac10b07914ecd04cb0e4de0d2d10a2b Mon Sep 17 00:00:00 2001
From: Michal Privoznik <mprivozn@redhat.com>
Date: Tue, 26 Jul 2022 19:27:06 +0200
Subject: [PATCH] conf: Validate virDomainMemoryDef::targetNode

Almost all of memory models we currently support allow setting
virDomainMemoryDef::targetNode so that the memory module is
associated with given guest NUMA node. And we do have a check
whether the requested node is within bounds, but it's executed
only when building QEMU's cmd line. Move it into validation
phase.

While this commit is moving the validation to a place that does
not validate all the possible code paths, it's okay, because only
the explicit memory device has user-configurable target node
which could break the assumption.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
---
 src/conf/domain_validate.c                    | 13 ++++++
 src/qemu/qemu_command.c                       |  9 ----
 .../memory-hotplug-invalid-targetnode.err     |  1 +
 .../memory-hotplug-invalid-targetnode.xml     | 42 +++++++++++++++++++
 tests/qemuxml2argvtest.c                      |  1 +
 5 files changed, 57 insertions(+), 9 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/memory-hotplug-invalid-targetnode.err
 create mode 100644 tests/qemuxml2argvdata/memory-hotplug-invalid-targetnode.xml

diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index cfd868fafa..e673e4b81a 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -2227,6 +2227,19 @@ virDomainMemoryDefValidate(const virDomainMemoryDef *mem,
 {
     unsigned long long thpSize;
 
+    /* Guest NUMA nodes are continuous and indexed from zero. */
+    if (mem->targetNode != -1) {
+        const size_t nodeCount = virDomainNumaGetNodeCount(def->numa);
+
+        if (mem->targetNode >= nodeCount) {
+            virReportError(VIR_ERR_XML_DETAIL,
+                           _("can't add memory backend for guest node '%d' as the guest has only '%zu' NUMA nodes configured"),
+                           mem->targetNode, nodeCount);
+            return -1;
+        }
+    }
+
+
     switch (mem->model) {
     case VIR_DOMAIN_MEMORY_MODEL_NVDIMM:
         if (!mem->nvdimmPath) {
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 30c9bbbf2e..77f5644b1c 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3695,15 +3695,6 @@ qemuBuildMemoryBackendProps(virJSONValue **backendProps,
     *backendProps = NULL;
 
     if (mem->targetNode >= 0) {
-        /* memory devices could provide a invalid guest node */
-        if (mem->targetNode >= virDomainNumaGetNodeCount(def->numa)) {
-            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                           _("can't add memory backend for guest node '%d' as "
-                             "the guest has only '%zu' NUMA nodes configured"),
-                           mem->targetNode, virDomainNumaGetNodeCount(def->numa));
-            return -1;
-        }
-
         if (memAccess == VIR_DOMAIN_MEMORY_ACCESS_DEFAULT)
             memAccess = virDomainNumaGetNodeMemoryAccessMode(def->numa, mem->targetNode);
 
diff --git a/tests/qemuxml2argvdata/memory-hotplug-invalid-targetnode.err b/tests/qemuxml2argvdata/memory-hotplug-invalid-targetnode.err
new file mode 100644
index 0000000000..a72e38bfaf
--- /dev/null
+++ b/tests/qemuxml2argvdata/memory-hotplug-invalid-targetnode.err
@@ -0,0 +1 @@
+can't add memory backend for guest node '1' as the guest has only '1' NUMA nodes configured
diff --git a/tests/qemuxml2argvdata/memory-hotplug-invalid-targetnode.xml b/tests/qemuxml2argvdata/memory-hotplug-invalid-targetnode.xml
new file mode 100644
index 0000000000..feb11368a8
--- /dev/null
+++ b/tests/qemuxml2argvdata/memory-hotplug-invalid-targetnode.xml
@@ -0,0 +1,42 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <maxMemory slots='16' unit='KiB'>1099511627776</maxMemory>
+  <memory unit='KiB'>1267710</memory>
+  <currentMemory unit='KiB'>1267710</currentMemory>
+  <vcpu placement='static' cpuset='0-1'>2</vcpu>
+  <os>
+    <type arch='x86_64' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <cpu>
+    <topology sockets='2' dies='1' cores='1' threads='1'/>
+    <numa>
+      <cell id='0' cpus='0-1' memory='219136' unit='KiB'/>
+    </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'/>
+    <controller type='usb' index='0'/>
+    <controller type='pci' index='0' model='pci-root'/>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <memballoon model='virtio'/>
+    <memory model='dimm'>
+      <target>
+        <size unit='KiB'>523264</size>
+        <node>1</node>
+      </target>
+    </memory>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index d1a36f653c..05537d9e96 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -3079,6 +3079,7 @@ mymain(void)
 
     DO_TEST_FAILURE_NOCAPS("memory-align-fail");
     DO_TEST_PARSE_ERROR("memory-hotplug-nonuma", QEMU_CAPS_DEVICE_PC_DIMM);
+    DO_TEST_PARSE_ERROR_NOCAPS("memory-hotplug-invalid-targetnode");
     DO_TEST_NOCAPS("memory-hotplug");
     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,
-- 
2.39.5