Re: [PATCH v1 1/4] qemu: revert latest pSeries NVDIMM design changes
On Mon, 2020-09-14 at 23:42 -0300, Daniel Henrique Barboza wrote:
In [1], changes were made to remove the existing auto-alignment
for pSeries NVDIMM devices. That design promotes strange situations
where the NVDIMM size reported in the domain XML is different
from what QEMU is actually using. We removed the auto-alignment
and relied on standard size validation.
However, this goes against Libvirt design philosophy of not
tampering with existing guest behavior, as pointed out by Daniel
in [2]. Since we can't know for sure whether there are guests that
are relying on the auto-alignment feature to work, the changes
made in [1] are a direct violation of this rule.
This patch reverts [1] entirely, re-enabling auto-alignment for
pSeries NVDIMM as it was before. Changes will be made to ease
the limitations of this design without hurting existing
guests.
This reverts the following commits:
- commit 0ee56369c8b4f2f898b6aa1ff1f51ab033be1c02,
"qemu_domain.c: change qemuDomainMemoryDeviceAlignSize() return type".
- commit 07de813924caf37e535855541c0c1183d9d382e2,
"qemu_domain.c: do not auto-align ppc64 NVDIMMs"
- commit 0ccceaa57c50e5ee528f7073fa8723afd62b88b7,
"qemu_validate.c: add pSeries NVDIMM size alignment validation"
- commit 4fa2202d884414ad34d9952e72fb39b1d93c7e14,
"qemu_domain.c: make qemuDomainGetMemorySizeAlignment() public"
- commit 2d93cbdea9d1b8dbf36bc0ffee6cb73d83d208c7,
"Revert "formatdomain.html.in: mention pSeries NVDIMM 'align down'
mechanic""
[1] https://www.redhat.com/archives/libvir-list/2020-July/msg02010.html
[2] https://www.redhat.com/archives/libvir-list/2020-September/msg00572.html
Reverting multiple commits with a single commit is sort of unusual,
but in this case it seem to make sense and I don't really have a
problem with it, so unless someone shouts against this approach I
think we can merge it as-is.
A few tweaks to the commit message, though:
* keep the order of commits consistent to the one they were merged
in, which more specifically means putting 2d93cbdea9d1 at the
very top of the list;
* lose the commas after the various commit hashes - they break
convenient double-click selection in most terminals;
* indent the commit subject by two spaces for better readability.
The commit is also missing your S-o-b, and as you know that's a
blocker for merging.
With the above addressed,
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization