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 <abolo...@redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
