virDomainMemoryDefValidate: Check for overlapping memory devices

As of v9.4.0-rc2~5 it is possible to specify guest address where
a virtio-mem/virtio-pmem memory device is mapped to. What that
commit forgot to introduce was a check for overlaps.

And yes, this is technically an O(n^2) algorithm, as
virDomainMemoryDefValidate() is called over each memory device
and after this, virDomainMemoryDefValidate() also iterates over
each memory device. But given there's usually only a handful of
such devices, and this runs only when parsing domain XML I guess
code readability wins over some less obvious solution.

Resolves: https://issues.redhat.com/browse/RHEL-4452
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index d14559cd7306019ac60ca3318af492b440253b7f..6962fe76bf6095aba7a5557ac8468fb1fbf41cb8 100644 (file)
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -2223,6 +2223,9 @@ virDomainMemoryDefValidate(const virDomainMemoryDef *mem,
 {
     const long pagesize = virGetSystemPageSize();
     unsigned long long thpSize;
+    unsigned long long thisStart = 0;
+    unsigned long long thisEnd = 0;
+    size_t i;
 
     /* Guest NUMA nodes are continuous and indexed from zero. */
     if (mem->targetNode != -1) {
@@ -2304,6 +2307,7 @@ virDomainMemoryDefValidate(const virDomainMemoryDef *mem,
                            pagesize);
             return -1;
         }
+        thisStart = mem->target.virtio_pmem.address;
         break;
 
     case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM:
@@ -2347,6 +2351,7 @@ virDomainMemoryDefValidate(const virDomainMemoryDef *mem,
                            _("memory device address must be aligned to blocksize"));
             return -1;
         }
+        thisStart = mem->target.virtio_mem.address;
         break;
 
     case VIR_DOMAIN_MEMORY_MODEL_DIMM:
@@ -2368,6 +2373,48 @@ virDomainMemoryDefValidate(const virDomainMemoryDef *mem,
         return -1;
     }
 
+    if (thisStart == 0) {
+        return 0;
+    }
+
+    /* thisStart and thisEnd are in bytes, mem->size in kibibytes */
+    thisEnd = thisStart + mem->size * 1024;
+
+    for (i = 0; i < def->nmems; i++) {
+        const virDomainMemoryDef *other = def->mems[i];
+        unsigned long long otherStart = 0;
+
+        if (other == mem)
+            continue;
+
+        switch (other->model) {
+        case VIR_DOMAIN_MEMORY_MODEL_NONE:
+        case VIR_DOMAIN_MEMORY_MODEL_DIMM:
+        case VIR_DOMAIN_MEMORY_MODEL_NVDIMM:
+        case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC:
+        case VIR_DOMAIN_MEMORY_MODEL_LAST:
+            continue;
+            break;
+
+        case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM:
+            otherStart = other->target.virtio_pmem.address;
+            break;
+        case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM:
+            otherStart = other->target.virtio_mem.address;
+            break;
+        }
+
+        if (otherStart == 0)
+            continue;
+
+        if (thisStart <= otherStart && thisEnd > otherStart) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("memory device address [0x%1$llx:0x%2$llx] overlaps with other memory device (0x%3$llx)"),
+                           thisStart, thisEnd, otherStart);
+            return -1;
+        }
+    }
+
     return 0;
 }
 

diff --git a/tests/qemuxml2argvdata/memory-hotplug-virtio-mem-overlap-address.x86_64-latest.err b/tests/qemuxml2argvdata/memory-hotplug-virtio-mem-overlap-address.x86_64-latest.err
new file mode 100644 (file)
index 0000000..36d5b8a
--- /dev/null
+++ b/tests/qemuxml2argvdata/memory-hotplug-virtio-mem-overlap-address.x86_64-latest.err
@@ -0,0 +1 @@
+unsupported configuration: memory device address [0x140000000:0x180000000] overlaps with other memory device (0x170000000)

diff --git a/tests/qemuxml2argvdata/memory-hotplug-virtio-mem-overlap-address.xml b/tests/qemuxml2argvdata/memory-hotplug-virtio-mem-overlap-address.xml
new file mode 100644 (file)
index 0000000..65999cc
--- /dev/null
+++ b/tests/qemuxml2argvdata/memory-hotplug-virtio-mem-overlap-address.xml
@@ -0,0 +1,50 @@
+<domain type='kvm'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <maxMemory unit='KiB'>1099511627776</maxMemory>
+  <memory unit='KiB'>8388608</memory>
+  <currentMemory unit='KiB'>8388608</currentMemory>
+  <vcpu placement='static' cpuset='0-1'>2</vcpu>
+  <os>
+    <type arch='x86_64' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <cpu mode='custom' match='exact' check='none'>
+    <model fallback='forbid'>qemu64</model>
+    <topology sockets='2' dies='1' cores='1' threads='1'/>
+    <numa>
+      <cell id='0' cpus='0-1' memory='2095104' 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>
+    <memory model='virtio-mem'>
+      <target>
+        <size unit='KiB'>1048576</size>
+        <node>0</node>
+        <block unit='KiB'>2048</block>
+        <requested unit='KiB'>524288</requested>
+        <address base='0x140000000'/>
+      </target>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
+    </memory>
+    <memory model='virtio-mem'>
+      <source>
+        <nodemask>1-3</nodemask>
+        <pagesize unit='KiB'>2048</pagesize>
+      </source>
+      <target>
+        <size unit='KiB'>2097152</size>
+        <node>0</node>
+        <block unit='KiB'>2048</block>
+        <requested unit='KiB'>1048576</requested>
+        <address base='0x170000000'/>
+      </target>
+      <address type='pci' domain='0x0000' bus='0x01' slot='0x01' function='0x0'/>
+    </memory>
+  </devices>
+</domain>

diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 2195d8efa34fcab75fac73ebb174b477ea2861ca..a454dcb205769c34c429b6faa99696b326cce9d4 100644 (file)
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2114,6 +2114,7 @@ mymain(void)
     DO_TEST_CAPS_LATEST("memory-hotplug-virtio-pmem");
     DO_TEST_CAPS_LATEST("memory-hotplug-virtio-mem");
     DO_TEST_CAPS_LATEST("memory-hotplug-multiple");
+    DO_TEST_CAPS_LATEST_PARSE_ERROR("memory-hotplug-virtio-mem-overlap-address");
 
     DO_TEST_CAPS_ARCH_LATEST("machine-aeskeywrap-on-caps", "s390x");
     DO_TEST_CAPS_ARCH_LATEST("machine-aeskeywrap-on-cap", "s390x");