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

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(a)gmail.com&gt; --- 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