[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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 redhat com>

-- 
Andrea Bolognani / Red Hat / Virtualization


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]