On 11/12/20 8:28 AM, Michal Privoznik wrote:
On 11/11/20 11:07 PM, Daniel Henrique Barboza wrote:
Hi,
This is a work that I intended to do after pSeries NVDIMM changes I made in commit v6.7.0-302-gd3f3c2c97f. The reasoning is that if ppc64 NVDIMM alignment can be done in PostParse time, regular ppc64 DIMMs alignment can also be done the same way. After that I realized that the same logic can be applied to x86 as well, and then I started to see where we could eliminate explicit align calls.
The end result is that a bug that affects pSeries the most was found and fixed in patch 02, and now we're able to reflect the exact memory being used by the guest in the live XML since the alignment is now done in parse time. The series can be fetched from:
https://gitlab.com/danielhb/libvirt/-/tree/memory_modules_postparse_v1
NOTE: I worked with the x86 logic based on what I could assert from the code itself, i.e. the aligment mechanics (2MiB align for mem modules, 1MiB align for total memory) are related to QEMU internals, not being hypervisor-agnostic. If someone with authority can guarantee that the alignment restrictions for x86 are applicable to all hypervisors (like in the pSeries case), let me know and I'll respin the series putting the x86 code in virDomainMemoryDefPostParse().
Daniel Henrique Barboza (10): domain_conf.c: handle all ppc64 dimms in virDomainNVDimmAlignSizePseries qemu_domain.c: align memory modules before calculating 'initialmem' domain_conf.c: align pSeries mem modules in virDomainMemoryDefPostParse() qemu_domain.c: simplify qemuDomainGetMemoryModuleSizeAlignment() qemu_domain.c: align x86 memory module in post parse time qemu_domain.c: move size check for mem modules after alignment qemu_hotplug.c: avoid aligning the mem obj in qemuDomainDetachPrepMemory() qemu_hotplug.c: avoid aligning the mem obj in qemuDomainAttachMemory() qemu_domain.c: remove qemuDomainMemoryDeviceAlignSize() NEWS.rs: document memory alignment improvements
NEWS.rst | 14 +++ src/conf/domain_conf.c | 25 ++++- src/conf/domain_conf.h | 2 +- src/libvirt_private.syms | 2 +- src/qemu/qemu_domain.c | 106 +++++++----------- src/qemu/qemu_domain.h | 2 - src/qemu/qemu_hotplug.c | 9 +- tests/qemuxml2argvdata/hugepages-nvdimm.xml | 2 +- .../memory-hotplug-nvdimm-access.xml | 2 +- .../memory-hotplug-nvdimm-align.xml | 2 +- .../memory-hotplug-nvdimm-label.xml | 2 +- .../memory-hotplug-nvdimm-pmem.xml | 2 +- .../memory-hotplug-nvdimm-readonly.xml | 2 +- .../memory-hotplug-nvdimm.xml | 2 +- .../memory-hotplug-ppc64-nonuma.args | 2 +- tests/qemuxml2argvdata/pages-dimm-discard.xml | 2 +- .../memory-hotplug-dimm.xml | 4 +- 17 files changed, 92 insertions(+), 90 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Thanks for the review!
but perhaps you want Andrea to look into it too.
Of course. Andrea, not sure if you want to review the whole series so here's the gist: we already have the ppc64 NVDIMM alignment being done in PostParse time, so I started to move all ppc64 memory module alignment to PostParse as well. In the end I figured I could also move the alignment for x86 as well. Do you see any potential problem with this design change? Thanks, DHB
Michal