From fe67e3e28e9887248610e6106d6245741acb7f96 Mon Sep 17 00:00:00 2001
From: Luyao Huang <lhuang@redhat.com>
Date: Mon, 20 Aug 2018 17:48:33 +0800
Subject: [PATCH] qemu: Validate memory access during validate domain config

Commit 6534b3c4 tried to raise an error when there is no numa
nodes by setting access='shared' in the domain config, but added
a helper called from qemuDomainDeviceDefValidate instead of a
helper called from qemuDomainDefValidate for XML:

  <memoryBacking>
    <hugepages/>
    <access mode='shared'/>
  </memoryBacking>

Since there are no memory devices in the test XML, there would
be no validation failure, but the test added was still failing.
Investigating that it turns out that unnecessary XML elements
were causing the failure (no need for <video>, <graphics>,
<pm>, usb controller model "piix3-uhci", disk attribute for
"discard='unmap'", <serial>, <console>, <channel> and a
memballoon model). Removing all those before moving the method
caused the test to succeed.

So this patch moves the validation to the right place and
removes all the unnecessary XML pieces that were causing
a false validation failure.

https://bugzilla.redhat.com/show_bug.cgi?id=1448149#c14

Signed-off-by: Luyao Huang <lhuang@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
---
 src/qemu/qemu_domain.c                        | 55 ++++++++--------
 .../qemuxml2argvdata/hugepages-memaccess3.xml | 62 +------------------
 tests/qemuxml2argvtest.c                      |  6 +-
 3 files changed, 32 insertions(+), 91 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index d052bf4ca8..e4ac58e95b 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3949,6 +3949,29 @@ qemuDomainDefValidateFeatures(const virDomainDef *def,
 }
 
 
+static int
+qemuDomainDefValidateMemory(const virDomainDef *def)
+{
+    const long system_page_size = virGetSystemPageSizeKB();
+
+    /* We can't guarantee any other mem.access
+     * if no guest NUMA nodes are defined. */
+    if (def->mem.nhugepages != 0 &&
+        def->mem.hugepages[0].size != system_page_size &&
+        virDomainNumaGetNodeCount(def->numa) == 0 &&
+        def->mem.access != VIR_DOMAIN_MEMORY_ACCESS_DEFAULT &&
+        def->mem.access != VIR_DOMAIN_MEMORY_ACCESS_PRIVATE) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("memory access mode '%s' not supported "
+                         "without guest numa node"),
+                       virDomainMemoryAccessTypeToString(def->mem.access));
+        return -1;
+    }
+
+    return 0;
+}
+
+
 static int
 qemuDomainDefValidate(const virDomainDef *def,
                       virCapsPtr caps ATTRIBUTE_UNUSED,
@@ -4071,6 +4094,9 @@ qemuDomainDefValidate(const virDomainDef *def,
     if (qemuDomainDefValidateFeatures(def, qemuCaps) < 0)
         goto cleanup;
 
+    if (qemuDomainDefValidateMemory(def) < 0)
+        goto cleanup;
+
     ret = 0;
 
  cleanup:
@@ -5599,30 +5625,6 @@ qemuDomainDeviceDefValidateController(const virDomainControllerDef *controller,
 }
 
 
-static int
-qemuDomainDeviceDefValidateMemory(const virDomainMemoryDef *memory ATTRIBUTE_UNUSED,
-                                  const virDomainDef *def)
-{
-    const long system_page_size = virGetSystemPageSizeKB();
-
-    /* We can't guarantee any other mem.access
-     * if no guest NUMA nodes are defined. */
-    if (def->mem.nhugepages != 0 &&
-        def->mem.hugepages[0].size != system_page_size &&
-        virDomainNumaGetNodeCount(def->numa) == 0 &&
-        def->mem.access != VIR_DOMAIN_MEMORY_ACCESS_DEFAULT &&
-        def->mem.access != VIR_DOMAIN_MEMORY_ACCESS_PRIVATE) {
-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                       _("memory access mode '%s' not supported "
-                         "without guest numa node"),
-                       virDomainMemoryAccessTypeToString(def->mem.access));
-        return -1;
-    }
-
-    return 0;
-}
-
-
 static int
 qemuDomainDeviceDefValidateVsock(const virDomainVsockDef *vsock,
                                  const virDomainDef *def,
@@ -5781,10 +5783,6 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev,
                                                     qemuCaps);
         break;
 
-    case VIR_DOMAIN_DEVICE_MEMORY:
-        ret = qemuDomainDeviceDefValidateMemory(dev->data.memory, def);
-        break;
-
     case VIR_DOMAIN_DEVICE_VSOCK:
         ret = qemuDomainDeviceDefValidateVsock(dev->data.vsock, def, qemuCaps);
         break;
@@ -5806,6 +5804,7 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev,
     case VIR_DOMAIN_DEVICE_MEMBALLOON:
     case VIR_DOMAIN_DEVICE_NVRAM:
     case VIR_DOMAIN_DEVICE_SHMEM:
+    case VIR_DOMAIN_DEVICE_MEMORY:
     case VIR_DOMAIN_DEVICE_PANIC:
     case VIR_DOMAIN_DEVICE_IOMMU:
     case VIR_DOMAIN_DEVICE_NONE:
diff --git a/tests/qemuxml2argvdata/hugepages-memaccess3.xml b/tests/qemuxml2argvdata/hugepages-memaccess3.xml
index 8ec38d802a..43c3ec688b 100644
--- a/tests/qemuxml2argvdata/hugepages-memaccess3.xml
+++ b/tests/qemuxml2argvdata/hugepages-memaccess3.xml
@@ -12,76 +12,18 @@
     <type arch='x86_64' machine='pc-i440fx-2.9'>hvm</type>
     <bootmenu enable='yes'/>
   </os>
-  <features>
-    <acpi/>
-    <apic/>
-    <pae/>
-  </features>
   <cpu mode='host-model' check='partial'>
     <model fallback='allow'/>
   </cpu>
-  <clock offset='variable' adjustment='500' basis='utc'>
-    <timer name='rtc' tickpolicy='catchup'/>
-    <timer name='pit' tickpolicy='delay'/>
-    <timer name='hpet' present='no'/>
-  </clock>
   <on_poweroff>destroy</on_poweroff>
   <on_reboot>restart</on_reboot>
   <on_crash>destroy</on_crash>
-  <pm>
-    <suspend-to-mem enabled='yes'/>
-    <suspend-to-disk enabled='yes'/>
-  </pm>
   <devices>
     <emulator>/usr/bin/qemu-system-x86_64</emulator>
-    <disk type='file' device='disk'>
-      <driver name='qemu' type='qcow2' discard='unmap'/>
-      <source file='/var/lib/libvirt/images/fedora.qcow2'/>
-      <target dev='sda' bus='scsi'/>
-      <boot order='1'/>
-      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
-    </disk>
-    <controller type='usb' index='0' model='piix3-uhci'>
-      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
-    </controller>
+    <controller type='usb' index='0'/>
     <controller type='pci' index='0' model='pci-root'/>
-    <controller type='scsi' index='0'>
-      <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/>
-    </controller>
-    <controller type='virtio-serial' index='0'>
-      <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/>
-    </controller>
-    <controller type='ide' index='0'>
-      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
-    </controller>
-    <serial type='pty'>
-      <target type='isa-serial' port='1'>
-        <model name='isa-serial'/>
-      </target>
-    </serial>
-    <console type='pty'>
-      <target type='serial' port='1'/>
-    </console>
-    <channel type='unix'>
-      <target type='virtio' name='org.qemu.guest_agent.0'/>
-      <address type='virtio-serial' controller='0' bus='0' port='1'/>
-    </channel>
     <input type='mouse' bus='ps2'/>
     <input type='keyboard' bus='ps2'/>
-    <graphics type='vnc' port='-1' autoport='yes' listen='0.0.0.0'>
-      <listen type='address' address='0.0.0.0'/>
-    </graphics>
-    <graphics type='spice'>
-      <listen type='socket' socket='/tmp/spice.sock'/>
-    </graphics>
-    <video>
-      <model type='virtio' heads='1' primary='yes'>
-        <acceleration accel3d='yes'/>
-      </model>
-      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
-    </video>
-    <memballoon model='virtio'>
-      <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/>
-    </memballoon>
+    <memballoon model='none'/>
   </devices>
 </domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 16c3540798..9bfe864597 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1023,9 +1023,9 @@ mymain(void)
     DO_TEST("hugepages-memaccess2", QEMU_CAPS_OBJECT_MEMORY_FILE,
             QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_DEVICE_PC_DIMM,
             QEMU_CAPS_NUMA);
-    DO_TEST_FAILURE("hugepages-memaccess3",
-            QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE,
-            QEMU_CAPS_VIRTIO_SCSI);
+    DO_TEST_PARSE_ERROR("hugepages-memaccess3",
+                        QEMU_CAPS_OBJECT_MEMORY_RAM,
+                        QEMU_CAPS_OBJECT_MEMORY_FILE);
     DO_TEST_CAPS_LATEST("hugepages-nvdimm");
     DO_TEST("nosharepages", QEMU_CAPS_MEM_MERGE);
     DO_TEST("disk-cdrom", NONE);
-- 
2.39.5