qemuBuildMemoryBackendStr: Handle one more corner case

https://bugzilla.redhat.com/show_bug.cgi?id=1458638

This code is so complicated because we allow enabling the same
bits at many places. Just like in this case: huge pages can be
enabled by global <hugepages/> element under <memoryBacking> or
on per <memory/> basis. To complicate things a bit more, users
are allowed to omit the page size which case the default page
size is used. And this is what is causing this bug. If no page
size is specified, @pagesize is keeping value of zero throughout
whole function. Therefore we need yet another boolean to hold
[use, don't use] information as we can't sue @pagesize for that.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 604650596324f296dd49775d4965d13e6b71647e..68fc137069a5b46afd8127b3733b4b8969fd5659 100644 (file)
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3250,8 +3250,6 @@ qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps,
                           virBitmapPtr autoNodeset,
                           bool force)
 {
-    virDomainHugePagePtr master_hugepage = NULL;
-    virDomainHugePagePtr hugepage = NULL;
     virDomainNumatuneMemMode mode;
     const long system_page_size = virGetSystemPageSizeKB();
     virDomainMemoryAccess memAccess = mem->access;
@@ -3264,6 +3262,13 @@ qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps,
     bool nodeSpecified = virDomainNumatuneNodeSpecified(def->numa, mem->targetNode);
     unsigned long long pagesize = mem->pagesize;
     bool needHugepage = !!pagesize;
+    bool useHugepage = !!pagesize;
+
+    /* The difference between @needHugepage and @useHugepage is that the latter
+     * is true whenever huge page is defined for the current memory cell.
+     * Either directly, or transitively via global domain huge pages. The
+     * former is true whenever "memory-backend-file" must be used to satisfy
+     * @useHugepage. */
 
     *backendProps = NULL;
     *backendType = NULL;
@@ -3290,6 +3295,8 @@ qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps,
         mode = VIR_DOMAIN_NUMATUNE_MEM_STRICT;
 
     if (pagesize == 0) {
+        virDomainHugePagePtr master_hugepage = NULL;
+        virDomainHugePagePtr hugepage = NULL;
         bool thisHugepage = false;
 
         /* Find the huge page size we want to use */
@@ -3327,8 +3334,10 @@ qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps,
             hugepage = master_hugepage;
         }
 
-        if (hugepage)
+        if (hugepage) {
             pagesize = hugepage->size;
+            useHugepage = true;
+        }
     }
 
     if (pagesize == system_page_size) {
@@ -3336,17 +3345,18 @@ qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps,
          * of regular system page size, it's as if they
          * hasn't specified any huge pages at all. */
         pagesize = 0;
-        hugepage = NULL;
+        needHugepage = false;
+        useHugepage = false;
     }
 
     if (!(props = virJSONValueNewObject()))
         return -1;
 
-    if (pagesize || mem->nvdimmPath || memAccess ||
+    if (useHugepage || mem->nvdimmPath || memAccess ||
         def->mem.source == VIR_DOMAIN_MEMORY_SOURCE_FILE) {
         *backendType = "memory-backend-file";
 
-        if (pagesize) {
+        if (useHugepage) {
             if (qemuGetDomainHupageMemPath(def, cfg, pagesize, &memPath) < 0)
                 goto cleanup;
             prealloc = true;

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages7.args b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages7.args
new file mode 100644 (file)
index 0000000..e4229dc
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages7.args
@@ -0,0 +1,35 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-x86_64 \
+-name fedora \
+-S \
+-M pc-i440fx-2.3 \
+-m size=1048576k,slots=16,maxmem=1099511627776k \
+-smp 2,sockets=2,cores=1,threads=1 \
+-mem-prealloc \
+-mem-path /dev/hugepages2M/libvirt/qemu/-1-fedora \
+-numa node,nodeid=0,cpus=0-1,mem=1024 \
+-object memory-backend-file,id=memdimm0,prealloc=yes,\
+mem-path=/dev/hugepages1G/libvirt/qemu/-1-fedora,size=1073741824,\
+host-nodes=1-3,policy=bind \
+-device pc-dimm,node=0,memdev=memdimm0,id=dimm0,slot=0 \
+-object memory-backend-file,id=memdimm1,prealloc=yes,\
+mem-path=/dev/hugepages2M/libvirt/qemu/-1-fedora,share=no,size=536870912 \
+-device pc-dimm,node=0,memdev=memdimm1,id=dimm1,slot=1 \
+-uuid 63840878-0deb-4095-97e6-fc444d9bc9fa \
+-nographic \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-fedora/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=readline \
+-boot c \
+-usb \
+-drive file=/var/lib/libvirt/images/fedora.qcow2,format=qcow2,if=none,\
+id=drive-virtio-disk0 \
+-device virtio-blk-pci,bus=pci.0,addr=0x7,drive=drive-virtio-disk0,\
+id=virtio-disk0 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages7.xml b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages7.xml
new file mode 100644 (file)
index 0000000..d75cf5a
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages7.xml
@@ -0,0 +1,65 @@
+<domain type='qemu'>
+  <name>fedora</name>
+  <uuid>63840878-0deb-4095-97e6-fc444d9bc9fa</uuid>
+  <maxMemory slots='16' unit='KiB'>1099511627776</maxMemory>
+  <memory unit='KiB'>2621439</memory>
+  <currentMemory unit='KiB'>2621439</currentMemory>
+  <memoryBacking>
+    <hugepages/>
+  </memoryBacking>
+  <vcpu placement='static'>2</vcpu>
+  <os>
+    <type arch='x86_64' machine='pc-i440fx-2.3'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <features>
+    <acpi/>
+    <apic/>
+    <pae/>
+  </features>
+  <cpu>
+    <numa>
+      <cell id='0' cpus='0-1' memory='1048576' unit='KiB'/>
+    </numa>
+  </cpu>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>restart</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-x86_64</emulator>
+    <disk type='file' device='disk'>
+      <driver name='qemu' type='qcow2'/>
+      <source file='/var/lib/libvirt/images/fedora.qcow2'/>
+      <target dev='vda' bus='virtio'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/>
+    </disk>
+    <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>
+    <memory model='dimm'>
+      <source>
+        <nodemask>1-3</nodemask>
+        <pagesize unit='KiB'>1048576</pagesize>
+      </source>
+      <target>
+        <size unit='KiB'>1048576</size>
+        <node>0</node>
+      </target>
+      <address type='dimm' slot='0'/>
+    </memory>
+    <memory model='dimm' access='private'>
+      <target>
+        <size unit='KiB'>524287</size>
+        <node>0</node>
+      </target>
+      <address type='dimm' slot='1'/>
+    </memory>
+  </devices>
+</domain>

diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 655c89bc3ed024f9b0bc462854d2e93ed937e95d..9938052310af73f3c2e2e8185e1da6c000396675 100644 (file)
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -844,6 +844,8 @@ mymain(void)
             QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE);
     DO_TEST("hugepages-pages5", QEMU_CAPS_MEM_PATH);
     DO_TEST("hugepages-pages6", NONE);
+    DO_TEST("hugepages-pages7", QEMU_CAPS_MEM_PATH,
+            QEMU_CAPS_DEVICE_PC_DIMM, QEMU_CAPS_OBJECT_MEMORY_FILE);
     DO_TEST("hugepages-memaccess", QEMU_CAPS_OBJECT_MEMORY_FILE,
             QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_DEVICE_PC_DIMM,
             QEMU_CAPS_NUMA);

diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-hugepages-pages7.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-hugepages-pages7.xml
new file mode 120000 (symlink)
index 0000000..9aafa6e
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-hugepages-pages7.xml
@@ -0,0 +1 @@
+../qemuxml2argvdata/qemuxml2argv-hugepages-pages7.xml
\ No newline at end of file

diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 0d549adecf93e50f9374bdd7e477f1202d1981da..311b7135651e0245f87d2b05cce623e3685c22fe 100644 (file)
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -433,6 +433,7 @@ mymain(void)
     DO_TEST("hugepages-pages4", NONE);
     DO_TEST("hugepages-pages5", NONE);
     DO_TEST("hugepages-pages6", NONE);
+    DO_TEST("hugepages-pages7", NONE);
     DO_TEST("hugepages-shared", NONE);
     DO_TEST("hugepages-memaccess", NONE);
     DO_TEST("hugepages-memaccess2", NONE);