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

[Daniel Henrique Barboza]

On 8/24/20 1:44 PM, Michal Privoznik wrote:
> 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 :-)


LOL this went completely unnoticed. This kind of stuff reveals my true
age :P


DHB



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