[libvirt] [PATCH 0/2] qemu: Improve memory alignment handling and fix upcoming powerpc

Peter Krempa (2): qemu: Make memory alignment helper more universal qemu: ppc64: Align memory sizes to 256MiB src/qemu/qemu_domain.c | 33 ++++++++++++++++------ src/qemu/qemu_domain.h | 3 +- src/qemu/qemu_hotplug.c | 4 +-- .../qemuxml2argv-pseries-cpu-compat.args | 2 +- 4 files changed, 30 insertions(+), 12 deletions(-) -- 2.4.5

Extract the size determination into a separate function and reuse it across the memory device alignment functions. Since later we will need to decide the alignment size according to architecture let's pass def to the functions. --- src/qemu/qemu_domain.c | 26 ++++++++++++++++++-------- src/qemu/qemu_domain.h | 3 ++- src/qemu/qemu_hotplug.c | 4 ++-- 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 8b050a0..c1b3326 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3110,30 +3110,39 @@ qemuDomainAgentAvailable(virDomainObjPtr vm, } +static unsigned long long +qemuDomainGetMemorySizeAlignment(virDomainDefPtr def ATTRIBUTE_UNUSED) +{ + /* Align memory size. QEMU requires rounding to next 4KiB block. + * We'll take the "traditional" path and round it to 1MiB*/ + + return 1024; +} + + int qemuDomainAlignMemorySizes(virDomainDefPtr def) { unsigned long long mem; + unsigned long long align = qemuDomainGetMemorySizeAlignment(def); size_t ncells = virDomainNumaGetNodeCount(def->numa); size_t i; /* align NUMA cell sizes if relevant */ for (i = 0; i < ncells; i++) { mem = virDomainNumaGetNodeMemorySize(def->numa, i); - virDomainNumaSetNodeMemorySize(def->numa, i, VIR_ROUND_UP(mem, 1024)); + virDomainNumaSetNodeMemorySize(def->numa, i, VIR_ROUND_UP(mem, align)); } /* align initial memory size */ mem = virDomainDefGetMemoryInitial(def); - virDomainDefSetMemoryInitial(def, VIR_ROUND_UP(mem, 1024)); + virDomainDefSetMemoryInitial(def, VIR_ROUND_UP(mem, align)); - /* Align maximum memory size. QEMU requires rounding to next 4KiB block. - * We'll take the "traditional" path and round it to 1MiB*/ - def->mem.max_memory = VIR_ROUND_UP(def->mem.max_memory, 1024); + def->mem.max_memory = VIR_ROUND_UP(def->mem.max_memory, align); /* Align memory module sizes */ for (i = 0; i < def->nmems; i++) - qemuDomainMemoryDeviceAlignSize(def->mems[i]); + def->mems[i]->size = VIR_ROUND_UP(def->mems[i]->size, align); return 0; } @@ -3148,9 +3157,10 @@ qemuDomainAlignMemorySizes(virDomainDefPtr def) * size so this should be safe). */ void -qemuDomainMemoryDeviceAlignSize(virDomainMemoryDefPtr mem) +qemuDomainMemoryDeviceAlignSize(virDomainDefPtr def, + virDomainMemoryDefPtr mem) { - mem->size = VIR_ROUND_UP(mem->size, 1024); + mem->size = VIR_ROUND_UP(mem->size, qemuDomainGetMemorySizeAlignment(def)); } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 2af7c59..db483fc 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -462,7 +462,8 @@ bool qemuDomainHasBlockjob(virDomainObjPtr vm, bool copy_only) ATTRIBUTE_NONNULL(1); int qemuDomainAlignMemorySizes(virDomainDefPtr def); -void qemuDomainMemoryDeviceAlignSize(virDomainMemoryDefPtr mem); +void qemuDomainMemoryDeviceAlignSize(virDomainDefPtr def, + virDomainMemoryDefPtr mem); virDomainChrSourceDefPtr qemuFindAgentConfig(virDomainDefPtr def); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 1ea397f..c34475e 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1772,7 +1772,7 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, if (!(devstr = qemuBuildMemoryDeviceStr(mem, vm->def, priv->qemuCaps))) goto cleanup; - qemuDomainMemoryDeviceAlignSize(mem); + qemuDomainMemoryDeviceAlignSize(vm->def, mem); if (qemuBuildMemoryBackendStr(mem->size, mem->pagesize, mem->targetNode, mem->sourceNodes, NULL, @@ -4253,7 +4253,7 @@ qemuDomainDetachMemoryDevice(virQEMUDriverPtr driver, return -1; } - qemuDomainMemoryDeviceAlignSize(memdef); + qemuDomainMemoryDeviceAlignSize(vm->def, memdef); if ((idx = virDomainMemoryFindByDef(vm->def, memdef)) < 0) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", -- 2.4.5

Since qemu upstream commit TBD [1] the memory size for numa nodes, memory devices and the total size needs to be aligned to multiples of 256MiB. Otherwise qemu refuses to start with the following message: qemu-kvm: Can't support memory configuration where RAM size 0x7d000000 or maxmem size 0x7d000000 isn't aligned to 256 MB Change the alignment code to round sizes to 256 MiB multiples. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1249006 ---- [1] Since qemu is in freeze, the commit that introduces the problem doesn't have a valid upstream commit ID yet. https://github.com/dgibson/qemu/commit/456156ea4974eeff761d3fe7addc279f297e2... --- src/qemu/qemu_domain.c | 9 ++++++++- tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat.args | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c1b3326..aad912a 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3111,11 +3111,18 @@ qemuDomainAgentAvailable(virDomainObjPtr vm, static unsigned long long -qemuDomainGetMemorySizeAlignment(virDomainDefPtr def ATTRIBUTE_UNUSED) +qemuDomainGetMemorySizeAlignment(virDomainDefPtr def) { /* Align memory size. QEMU requires rounding to next 4KiB block. * We'll take the "traditional" path and round it to 1MiB*/ + /* pseries aligns memory to 256MiB chunks */ + if ((ARCH_IS_PPC64(def->os.arch) && + def->os.machine && + STRPREFIX(def->os.machine, "pseries"))) { + return 256 * 1024; + } + return 1024; } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat.args b/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat.args index 64df406..305e924 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat.args @@ -1,7 +1,7 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \ QEMU_AUDIO_DRV=none /usr/bin/qemu-system-ppc64 -S -M pseries \ -cpu host,compat=power7 \ --m 214 -smp 4 -nographic -nodefconfig -nodefaults \ +-m 256 -smp 4 -nographic -nodefconfig -nodefaults \ -chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait \ -mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -usb \ -chardev pty,id=charserial0 \ -- 2.4.5

On Fri, 2015-07-31 at 16:49 +0200, Peter Krempa wrote:
Peter Krempa (2): qemu: Make memory alignment helper more universal qemu: ppc64: Align memory sizes to 256MiB
src/qemu/qemu_domain.c | 33 ++++++++++++++++------ src/qemu/qemu_domain.h | 3 +- src/qemu/qemu_hotplug.c | 4 +-- .../qemuxml2argv-pseries-cpu-compat.args | 2 +- 4 files changed, 30 insertions(+), 12 deletions(-)
Looks okay overall and works as expected from my limited testing on ppc64. One thing, though: the domain XML is not updated with the rounded-up value, which I think might be a little bit confusing to the user. Any chance the rounding up could be reflected there as well? Cheers. -- Andrea Bolognani Software Engineer - Virtualization Team

On Mon, Aug 03, 2015 at 11:02:49 +0200, Andrea Bolognani wrote:
On Fri, 2015-07-31 at 16:49 +0200, Peter Krempa wrote:
Peter Krempa (2): qemu: Make memory alignment helper more universal qemu: ppc64: Align memory sizes to 256MiB
src/qemu/qemu_domain.c | 33 ++++++++++++++++------ src/qemu/qemu_domain.h | 3 +- src/qemu/qemu_hotplug.c | 4 +-- .../qemuxml2argv-pseries-cpu-compat.args | 2 +- 4 files changed, 30 insertions(+), 12 deletions(-)
Looks okay overall and works as expected from my limited testing on ppc64.
One thing, though: the domain XML is not updated with the rounded-up value, which I think might be a little bit confusing to the user. Any chance the rounding up could be reflected there as well?
The rounding will be reflected once you start the VM in the live XML. This is also the case with the 1MiB rounding that is currently used, but it's not as prominent as the 256MiB granularity. Peter

On Mon, 2015-08-03 at 15:13 +0200, Peter Krempa wrote:
Looks okay overall and works as expected from my limited testing on ppc64.
One thing, though: the domain XML is not updated with the rounded-up value, which I think might be a little bit confusing to the user. Any chance the rounding up could be reflected there as well?
The rounding will be reflected once you start the VM in the live XML. This is also the case with the 1MiB rounding that is currently used, but it's not as prominent as the 256MiB granularity.
Let's file updating the on-disk XML under "would be nice to have" then. ACK series. Cheers. -- Andrea Bolognani Software Engineer - Virtualization Team
participants (2)
-
Andrea Bolognani
-
Peter Krempa