[PATCH v3 3/5] domain_conf.c: revert auto-alignment of pSeries NVDIMM in post parse

[Andrea Bolognani]

On Fri, 2020-12-04 at 13:50 -0300, Daniel Henrique Barboza wrote:
> On 12/4/20 12:13 PM, Andrea Bolognani wrote:
> > I suggest you instead do a pseudo-revert, where you rename the
> > function (without otherwise altering its signature) and move it to
> > the part of qemu_domain.c where you are ultimately going to need it;
> > in the commit message, you can still mention the commit that such a
> > change is a "spiritual revert" of, but this way we avoid muddying the
> > waters more than necessary.
> 
> The change in signature was done to avoid using qemuDomainGetMemorySizeAlignment(def),
> because that would be another function that would need to be moved up. I forgot to
> mention that in patch 04.

Why would you want to avoid using it? It's exactly what that function
is intended to do. Moving it around is no big deal, and by using it
you can avoid open-coding the same value twice. See attached diff.

> IIUC what you're suggesting can be implemented as follows:
> 
> - go back to the first revert I was doing in v2 (remove the code from PostParse).
> Single revert of d3f3c2c97f9b92;
> 
> - apply the other 2 patches;
> 
> - do an extra patch to rename/move the function that is now being used only
> in QEMU files, but without a straight up revert of ace5931553c8. I can also
> simplify the signature in this step.

Yes, except of course the other two patches should still include the
changes that were implemented after my review, eg. they should be as
seen in

  https://gitlab.com/abologna/libvirt/-/tree/ppc64-memalign

and not how they showed up initially on the list.

> The order here is intentional - the double revert in patch 3 done in this series
> needed to be followed up by changes in patches 4 and 5 (given that the function was
> renamed in patch 3). In this order, I can pick the patches from your local branch
> directly and just bother with the renaming in a single follow-up patch.
> 
> 
> Sounds good?

Yeah, it sounds like a plan. You can consider all patches from the
branch linked above

  Reviewed-by: Andrea Bolognani <abologna at redhat.com>

and push them at your leisure.

-- 
Andrea Bolognani / Red Hat / Virtualization
-------------- next part --------------
A non-text attachment was scrubbed...
Name: align-size.diff
Type: text/x-patch
Size: 5398 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20201204/d2f72ab2/attachment-0001.bin>