[PATCH v1 02/10] qemu_domain.c: align memory modules before calculating 'initialmem'

[Daniel Henrique Barboza]

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.

This behavior was found when trying to push ppc64 memory alignment
to postparse, as done in the next patch. In that case, the memory
modules are aligned before entering qemuDomainAlignMemorySizes() and
the 'memory-hotplug-ppc64-nonuma' test started to fail.

Fortunately the fix is simple: align the memory modules before
calculating 'initialmem' in qemuDomainAlignMemorySizes().

Signed-off-by: Daniel Henrique Barboza <danielhb413 at gmail.com>
---
 src/qemu/qemu_domain.c                        | 42 ++++++++++---------
 .../memory-hotplug-ppc64-nonuma.args          |  2 +-
 2 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 375a9e5075..5a17887fa1 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8083,25 +8083,9 @@ qemuDomainAlignMemorySizes(virDomainDefPtr def)
         virDomainNumaSetNodeMemorySize(def->numa, i, mem);
     }
 
-    /* align initial memory size, if NUMA is present calculate it as total of
-     * individual aligned NUMA node sizes */
-    if (initialmem == 0)
-        initialmem = VIR_ROUND_UP(virDomainDefGetMemoryInitial(def), align);
-
-    if (initialmem > maxmemcapped) {
-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                       _("initial memory size overflowed after alignment"));
-        return -1;
-    }
-
-    def->mem.max_memory = VIR_ROUND_UP(def->mem.max_memory, align);
-    if (def->mem.max_memory > maxmemkb) {
-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                       _("maximum memory size overflowed after alignment"));
-        return -1;
-    }
-
-    /* Align memory module sizes */
+    /* Align memory module sizes. This needs to occur before 'initialmem'
+     * calculation because virDomainDefGetMemoryInitial() uses the size
+     * of the modules in the math. */
     for (i = 0; i < def->nmems; i++) {
         if (def->mems[i]->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM &&
             ARCH_IS_PPC64(def->os.arch)) {
@@ -8122,6 +8106,26 @@ qemuDomainAlignMemorySizes(virDomainDefPtr def)
         }
     }
 
+    /* Align initial memory size, if NUMA is present calculate it as total of
+     * individual aligned NUMA node sizes. */
+    if (initialmem == 0) {
+        align = qemuDomainGetMemorySizeAlignment(def);
+        initialmem = VIR_ROUND_UP(virDomainDefGetMemoryInitial(def), align);
+    }
+
+    if (initialmem > maxmemcapped) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                       _("initial memory size overflowed after alignment"));
+        return -1;
+    }
+
+    def->mem.max_memory = VIR_ROUND_UP(def->mem.max_memory, align);
+    if (def->mem.max_memory > maxmemkb) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                       _("maximum memory size overflowed after alignment"));
+        return -1;
+    }
+
     virDomainDefSetMemoryTotal(def, initialmem + hotplugmem);
 
     return 0;
diff --git a/tests/qemuxml2argvdata/memory-hotplug-ppc64-nonuma.args b/tests/qemuxml2argvdata/memory-hotplug-ppc64-nonuma.args
index 91cea9d8bf..78406f7f04 100644
--- a/tests/qemuxml2argvdata/memory-hotplug-ppc64-nonuma.args
+++ b/tests/qemuxml2argvdata/memory-hotplug-ppc64-nonuma.args
@@ -11,7 +11,7 @@ QEMU_AUDIO_DRV=none \
 -name QEMUGuest1 \
 -S \
 -machine pseries,accel=kvm,usb=off,dump-guest-core=off \
--m size=1310720k,slots=16,maxmem=4194304k \
+-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 \
-- 
2.26.2