[PATCH 2/5] qemu_validate.c: add pSeries NVDIMM size alignment validation

[Michal Privoznik]

On 7/30/20 9:48 PM, Daniel Henrique Barboza wrote:
> The existing auto-align behavior for pSeries has the idea to
> alleviate user configuration of the NVDIMM size, given that the
> alignment calculation is not trivial to do (256MiB alignment
> of mem->size - mem->label_size value, a.k.a guest area). We
> align mem->size down to avoid end of file problems.
> 
> The end result is not ideal though. We do not touch the domain
> XML, meaning that the XML can report a NVDIMM size 255MiB smaller
> than the actual size the guest is seeing. It also adds one more
> thing to consider in case the guest is reporting less memory
> than declared, since the auto-align is transparent to the
> user.
> 
> Following Andrea's suggestion in [1], let's instead do an
> size alignment validation. If the NVDIMM is unaligned, error out
> and suggest a rounded up value. This can be bothersome to users,
> but will bring consistency of NVDIMM size between the domain XML
> and the guest.
> 
> This approach will force existing non-running pSeries guests to
> readjust the NVDIMM value in their XMLs, if necessary. No changes
> were made for x86 NVDIMM support.
> 
> [1] https://www.redhat.com/archives/libvir-list/2020-July/msg01471.html
> 
> Suggested-by: Andrea Bolognani <abologna at redhat.com>
> Signed-off-by: Daniel Henrique Barboza <danielhb413 at gmail.com>
> ---
>   src/qemu/qemu_validate.c                      | 43 ++++++++++++++++---
>   ...ory-hotplug-nvdimm-ppc64.ppc64-latest.args |  2 +-
>   .../memory-hotplug-nvdimm-ppc64.xml           |  2 +-
>   .../memory-hotplug-nvdimm-ppc64.xml           |  2 +-
>   4 files changed, 40 insertions(+), 9 deletions(-)
> 
> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> index 488f258d00..241139e268 100644
> --- a/src/qemu/qemu_validate.c
> +++ b/src/qemu/qemu_validate.c
> @@ -4012,15 +4012,46 @@ qemuValidateDomainDeviceDefHub(virDomainHubDefPtr hub,
>   }
>   
>   
> +static unsigned long long
> +qemuValidateGetNVDIMMAlignedSizePseries(virDomainMemoryDefPtr mem,
> +                                        const virDomainDef *def)
> +{
> +    unsigned long long ppc64AlignSize =
> +        qemuDomainGetMemorySizeAlignment((virDomainDefPtr)def);

Ouch. This is not nice. We need a trivial patch that makes 
qemuDomainGetMemorySizeAlignment() accept a const virDomainDef. I'm 
placing it at the beginning before 1/5.

> +    unsigned long long guestArea = mem->size - mem->labelsize;
> +
> +    // NVDIMM is already aligned

We're old style, we use c89 style of comments :-)

> +    if (guestArea % ppc64AlignSize == 0)
> +        return mem->size;
> +
> +    // Suggested aligned size is rounded up
> +    guestArea = (guestArea/ppc64AlignSize + 1) * ppc64AlignSize;
> +    return guestArea + mem->labelsize;
> +}

Michal