[PATCH v2 6/6] qemu_domain.c: align all pSeries mem modules when PARSE_ABI_UPDATE

Daniel Henrique Barboza posted 6 patches 2 months ago

[PATCH v2 6/6] qemu_domain.c: align all pSeries mem modules when PARSE_ABI_UPDATE

Posted by Daniel Henrique Barboza 2 months ago
qemuDomainAlignMemorySizes() has an operation order problem. We are
calculating 'initialmem' without aligning the memory modules first.
Since we're aligning the dimms afterwards this can create inconsistencies
in the end result. x86 has alignment of 1-2MiB and it's not severely
impacted by it, but pSeries works with 256MiB alignment and the difference
is noticeable.

This is the case of the existing 'memory-hotplug-ppc64-nonuma' test.
The test consists of a 2GiB (aligned value) guest with 2 ~520MiB dimms,
both unaligned. 'initialmem' is calculated by taking total_mem and
subtracting the dimms size (via virDomainDefGetMemoryInitial()), which
wil give us 2GiB - 520MiB - 520MiB, ending up with a little more than
an 1GiB of 'initialmem'. Note that this value is now unaligned, and
will be aligned up via VIR_ROUND_UP(), and we'll end up with 'initialmem'
of 1GiB + 256MiB. Given that the dimms are aligned later on, the end
result for QEMU is that the guest will have a 'mem' size of 1310720k,
plus the two 512 MiB dimms, exceeding in 256MiB the desired 2GiB
memory and currentMemory specified in the XML.

Existing guests can't be fixed without breaking ABI, but we have
code already in place to align pSeries NVDIMM modules for new guests.
Let's extend it to align all pSeries mem modules.

A new test, 'memory-hotplug-ppc64-nonuma-abi-update', a copy of the
existing 'memory-hotplug-ppc64-nonuma', was added to demonstrate the
result for new pSeries guests. For the same unaligned XML mentioned
above, after applying this patch:

- starting QEMU mem size without PARSE_ABI_UPDATE:
    -m size=1310720k,slots=16,maxmem=4194304k \ (no changes)

- starting QEMU mem size with PARSE_ABI_UPDATE:
    -m size=1048576k,slots=16,maxmem=4194304k \ (size fixed)

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 src/qemu/qemu_domain.c                        | 14 ++++--
 ...emory-hotplug-ppc64-nonuma-abi-update.args | 34 ++++++++++++++
 ...memory-hotplug-ppc64-nonuma-abi-update.xml | 32 +++++++++++++
 tests/qemuxml2argvtest.c                      |  7 +++
 ...memory-hotplug-ppc64-nonuma-abi-update.xml | 45 +++++++++++++++++++
 tests/qemuxml2xmltest.c                       |  7 +++
 6 files changed, 135 insertions(+), 4 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/memory-hotplug-ppc64-nonuma-abi-update.args
 create mode 100644 tests/qemuxml2argvdata/memory-hotplug-ppc64-nonuma-abi-update.xml
 create mode 100644 tests/qemuxml2xmloutdata/memory-hotplug-ppc64-nonuma-abi-update.xml

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index b9eb54a11c..a16ec9ac58 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5354,10 +5354,16 @@ qemuDomainMemoryDefPostParse(virDomainMemoryDefPtr mem, virArch arch,
      * later on by qemuDomainAlignMemorySizes() to contemplate existing
      * guests as well. */
     if (parseFlags & VIR_DOMAIN_DEF_PARSE_ABI_UPDATE) {
-        if (ARCH_IS_PPC64(arch) &&
-            mem->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM &&
-            virDomainNVDimmAlignSizePseries(mem) < 0)
-            return -1;
+        if (ARCH_IS_PPC64(arch)) {
+            unsigned long long ppc64MemModuleAlign = 256 * 1024;
+
+            if (mem->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM) {
+                if (virDomainNVDimmAlignSizePseries(mem) < 0)
+                    return -1;
+            } else {
+                mem->size = VIR_ROUND_UP(mem->size, ppc64MemModuleAlign);
+            }
+        }
     }
 
     return 0;
diff --git a/tests/qemuxml2argvdata/memory-hotplug-ppc64-nonuma-abi-update.args b/tests/qemuxml2argvdata/memory-hotplug-ppc64-nonuma-abi-update.args
new file mode 100644
index 0000000000..78406f7f04
--- /dev/null
+++ b/tests/qemuxml2argvdata/memory-hotplug-ppc64-nonuma-abi-update.args
@@ -0,0 +1,34 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-QEMUGuest1 \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-ppc64 \
+-name QEMUGuest1 \
+-S \
+-machine pseries,accel=kvm,usb=off,dump-guest-core=off \
+-m size=1048576k,slots=16,maxmem=4194304k \
+-realtime mlock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-object memory-backend-ram,id=memdimm0,size=536870912 \
+-device pc-dimm,memdev=memdimm0,id=dimm0,slot=0 \
+-object memory-backend-ram,id=memdimm1,size=536870912 \
+-device pc-dimm,memdev=memdimm1,id=dimm1,slot=1 \
+-uuid 49545eb3-75e1-2d0a-acdd-f0294406c99e \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-kernel /media/ram/uImage \
+-initrd /media/ram/ramdisk \
+-append 'root=/dev/ram rw console=ttyS0,115200' \
+-usb \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2
diff --git a/tests/qemuxml2argvdata/memory-hotplug-ppc64-nonuma-abi-update.xml b/tests/qemuxml2argvdata/memory-hotplug-ppc64-nonuma-abi-update.xml
new file mode 100644
index 0000000000..7c68cd6aa2
--- /dev/null
+++ b/tests/qemuxml2argvdata/memory-hotplug-ppc64-nonuma-abi-update.xml
@@ -0,0 +1,32 @@
+<domain type='kvm'>
+  <name>QEMUGuest1</name>
+  <uuid>49545eb3-75e1-2d0a-acdd-f0294406c99e</uuid>
+  <maxMemory slots='16' unit='KiB'>4194304</maxMemory>
+  <memory unit='KiB'>2097152</memory>
+  <currentMemory unit='KiB'>2097152</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='ppc64' machine='pseries'>hvm</type>
+    <kernel>/media/ram/uImage</kernel>
+    <initrd>/media/ram/ramdisk</initrd>
+    <cmdline>root=/dev/ram rw console=ttyS0,115200</cmdline>
+  </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-ppc64</emulator>
+    <memballoon model='virtio'/>
+    <memory model='dimm'>
+      <target>
+        <size unit='KiB'>523264</size>
+      </target>
+    </memory>
+    <memory model='dimm'>
+      <target>
+        <size unit='KiB'>524287</size>
+      </target>
+    </memory>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 174294c0f1..dabf00a53a 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -3005,6 +3005,13 @@ mymain(void)
     DO_TEST("memory-hotplug-ppc64-nonuma", QEMU_CAPS_KVM, QEMU_CAPS_DEVICE_PC_DIMM, QEMU_CAPS_NUMA,
             QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE);
+    DO_TEST_FULL("memory-hotplug-ppc64-nonuma-abi-update",
+                 ARG_PARSEFLAGS, VIR_DOMAIN_DEF_PARSE_ABI_UPDATE,
+                 ARG_QEMU_CAPS,
+                 QEMU_CAPS_KVM, QEMU_CAPS_DEVICE_PC_DIMM,
+                 QEMU_CAPS_NUMA, QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
+                 QEMU_CAPS_OBJECT_MEMORY_RAM,
+                 QEMU_CAPS_OBJECT_MEMORY_FILE);
     DO_TEST_CAPS_LATEST("memory-hotplug-nvdimm");
     DO_TEST_CAPS_LATEST("memory-hotplug-nvdimm-access");
     DO_TEST_CAPS_LATEST("memory-hotplug-nvdimm-label");
diff --git a/tests/qemuxml2xmloutdata/memory-hotplug-ppc64-nonuma-abi-update.xml b/tests/qemuxml2xmloutdata/memory-hotplug-ppc64-nonuma-abi-update.xml
new file mode 100644
index 0000000000..832c4cea27
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/memory-hotplug-ppc64-nonuma-abi-update.xml
@@ -0,0 +1,45 @@
+<domain type='kvm'>
+  <name>QEMUGuest1</name>
+  <uuid>49545eb3-75e1-2d0a-acdd-f0294406c99e</uuid>
+  <maxMemory slots='16' unit='KiB'>4194304</maxMemory>
+  <memory unit='KiB'>2097152</memory>
+  <currentMemory unit='KiB'>2097152</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='ppc64' machine='pseries'>hvm</type>
+    <kernel>/media/ram/uImage</kernel>
+    <initrd>/media/ram/ramdisk</initrd>
+    <cmdline>root=/dev/ram rw console=ttyS0,115200</cmdline>
+    <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-ppc64</emulator>
+    <controller type='usb' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
+    </controller>
+    <controller type='pci' index='0' model='pci-root'>
+      <model name='spapr-pci-host-bridge'/>
+      <target index='0'/>
+    </controller>
+    <memballoon model='virtio'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
+    </memballoon>
+    <panic model='pseries'/>
+    <memory model='dimm'>
+      <target>
+        <size unit='KiB'>524288</size>
+      </target>
+      <address type='dimm' slot='0'/>
+    </memory>
+    <memory model='dimm'>
+      <target>
+        <size unit='KiB'>524288</size>
+      </target>
+      <address type='dimm' slot='1'/>
+    </memory>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 595a897a70..f20616fdac 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1246,6 +1246,13 @@ mymain(void)
                  QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
                  QEMU_CAPS_DEVICE_NVDIMM,
                  QEMU_CAPS_LAST);
+    DO_TEST_FULL("memory-hotplug-ppc64-nonuma-abi-update", WHEN_BOTH,
+                 ARG_PARSEFLAGS, VIR_DOMAIN_DEF_PARSE_ABI_UPDATE,
+                 ARG_QEMU_CAPS,
+                 QEMU_CAPS_KVM, QEMU_CAPS_DEVICE_PC_DIMM,
+                 QEMU_CAPS_NUMA, QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
+                 QEMU_CAPS_OBJECT_MEMORY_RAM,
+                 QEMU_CAPS_OBJECT_MEMORY_FILE, QEMU_CAPS_LAST);
 
     DO_TEST("net-udp", NONE);
 
-- 
2.26.2

Re: [PATCH v2 6/6] qemu_domain.c: align all pSeries mem modules when PARSE_ABI_UPDATE

Posted by Andrea Bolognani 1 month, 2 weeks ago
On Wed, 2020-11-18 at 16:58 -0300, Daniel Henrique Barboza wrote:
> +++ b/tests/qemuxml2argvdata/memory-hotplug-ppc64-nonuma-abi-update.xml
> @@ -0,0 +1,32 @@
> +  <os>
> +    <type arch='ppc64' machine='pseries'>hvm</type>
> +    <kernel>/media/ram/uImage</kernel>
> +    <initrd>/media/ram/ramdisk</initrd>
> +    <cmdline>root=/dev/ram rw console=ttyS0,115200</cmdline>
> +  </os>

This can also be simplified. Once again, please look at

  https://www.redhat.com/archives/libvir-list/2020-December/msg00244.html

-- 
Andrea Bolognani / Red Hat / Virtualization