[PATCH 0/5] remove NVDIMM auto-alignment for pSeries guests
Hi, In [1] Andrea proposed that we should *not* auto-align down the NVDIMM size for pSeries guests. Instead we should error out and provide users with a suggested aligned value. This patch implements it. [1] https://www.redhat.com/archives/libvir-list/2020-July/msg01471.html Daniel Henrique Barboza (5): qemu_domain.c: make qemuDomainGetMemorySizeAlignment() public qemu_validate.c: add pSeries NVDIMM size alignment validation qemu_domain.c: do not auto-align ppc64 NVDIMMs qemu_domain.c: change qemuDomainMemoryDeviceAlignSize() return type Revert "formatdomain.html.in: mention pSeries NVDIMM 'align down' mechanic" docs/formatdomain.html.in | 6 +- src/qemu/qemu_domain.c | 57 ++----------------- src/qemu/qemu_domain.h | 7 ++- src/qemu/qemu_hotplug.c | 6 +- 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 +- 8 files changed, 55 insertions(+), 70 deletions(-) -- 2.26.2
Next patch will use it outside of qemu_domain.c. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_domain.h | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 2058290870..dfe3c3b64b 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7939,7 +7939,7 @@ qemuDomainAgentAvailable(virDomainObjPtr vm, } -static unsigned long long +unsigned long long qemuDomainGetMemorySizeAlignment(virDomainDefPtr def) { /* PPC requires the memory sizes to be rounded to 256MiB increments, so diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 29849a7313..cbd56861da 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -989,6 +989,9 @@ qemuDomainFixupCPUs(virDomainObjPtr vm, char * qemuDomainGetMachineName(virDomainObjPtr vm); +unsigned long long +qemuDomainGetMemorySizeAlignment(virDomainDefPtr def); + void qemuDomainObjPrivateXMLFormatAllowReboot(virBufferPtr buf, virTristateBool allowReboot); -- 2.26.2
On 7/30/20 9:48 PM, Daniel Henrique Barboza wrote:
Next patch will use it outside of qemu_domain.c.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_domain.h | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 2058290870..dfe3c3b64b 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7939,7 +7939,7 @@ qemuDomainAgentAvailable(virDomainObjPtr vm, }
-static unsigned long long +unsigned long long qemuDomainGetMemorySizeAlignment(virDomainDefPtr def) { /* PPC requires the memory sizes to be rounded to 256MiB increments, so diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 29849a7313..cbd56861da 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -989,6 +989,9 @@ qemuDomainFixupCPUs(virDomainObjPtr vm, char * qemuDomainGetMachineName(virDomainObjPtr vm);
+unsigned long long +qemuDomainGetMemorySizeAlignment(virDomainDefPtr def); + void qemuDomainObjPrivateXMLFormatAllowReboot(virBufferPtr buf, virTristateBool allowReboot);
Nitpick, this header declaration could be moved a couple of lines down, next to qemuDomainAlignMemorySizes() which is also close in .c file :-) Michal
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@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@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); + unsigned long long guestArea = mem->size - mem->labelsize; + + // NVDIMM is already aligned + if (guestArea % ppc64AlignSize == 0) + return mem->size; + + // Suggested aligned size is rounded up + guestArea = (guestArea/ppc64AlignSize + 1) * ppc64AlignSize; + return guestArea + mem->labelsize; +} + static int qemuValidateDomainDeviceDefMemory(virDomainMemoryDefPtr mem, + const virDomainDef *def, virQEMUCapsPtr qemuCaps) { - if (mem->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVDIMM)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("nvdimm isn't supported by this QEMU binary")); - return -1; + if (mem->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVDIMM)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("nvdimm isn't supported by this QEMU binary")); + return -1; + } + + if (qemuDomainIsPSeries(def)) { + unsigned long long alignedNVDIMMSize = + qemuValidateGetNVDIMMAlignedSizePseries(mem, def); + + if (mem->size != alignedNVDIMMSize) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("nvdimm size is not aligned. Suggested aligned " + "size: %llu KiB"), alignedNVDIMMSize); + return -1; + } + } } return 0; @@ -4138,7 +4169,7 @@ qemuValidateDomainDeviceDef(const virDomainDeviceDef *dev, break; case VIR_DOMAIN_DEVICE_MEMORY: - ret = qemuValidateDomainDeviceDefMemory(dev->data.memory, qemuCaps); + ret = qemuValidateDomainDeviceDefMemory(dev->data.memory, def, qemuCaps); break; case VIR_DOMAIN_DEVICE_LEASE: diff --git a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64.ppc64-latest.args b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64.ppc64-latest.args index eff80dcf80..58e3f9e161 100644 --- a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64.ppc64-latest.args +++ b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64.ppc64-latest.args @@ -19,7 +19,7 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -smp 2,sockets=2,dies=1,cores=1,threads=1 \ -numa node,nodeid=0,cpus=0-1,mem=1024 \ -object memory-backend-file,id=memnvdimm0,prealloc=yes,mem-path=/tmp/nvdimm,\ -size=537001984 \ +size=805437440 \ -device nvdimm,node=0,label-size=131072,\ uuid=49545eb3-75e1-2d0a-acdd-f0294406c99e,memdev=memnvdimm0,id=nvdimm0,slot=0 \ -uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ diff --git a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64.xml b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64.xml index ae5a17d3c8..10c146e8cf 100644 --- a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64.xml +++ b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64.xml @@ -38,7 +38,7 @@ <path>/tmp/nvdimm</path> </source> <target> - <size unit='KiB'>550000</size> + <size unit='KiB'>786560</size> <node>0</node> <label> <size unit='KiB'>128</size> diff --git a/tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-ppc64.xml b/tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-ppc64.xml index ae5a17d3c8..10c146e8cf 100644 --- a/tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-ppc64.xml +++ b/tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-ppc64.xml @@ -38,7 +38,7 @@ <path>/tmp/nvdimm</path> </source> <target> - <size unit='KiB'>550000</size> + <size unit='KiB'>786560</size> <node>0</node> <label> <size unit='KiB'>128</size> -- 2.26.2
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@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@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
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@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@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
On Thu, Jul 30, 2020 at 04:48:01PM -0300, 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.
This change violates on of the key requirements of libvirt design which is to never break existing guests that are running successfully. We must not assume users are ok with manually changing their guests during an upgrade scenario, so IMHO we need to revert this change. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 9/10/20 11:59 AM, Daniel P. Berrangé wrote:
On Thu, Jul 30, 2020 at 04:48:01PM -0300, 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.
This change violates on of the key requirements of libvirt design which is to never break existing guests that are running successfully. We must not assume users are ok with manually changing their guests during an upgrade scenario, so IMHO we need to revert this change.
I think this approach is cleaner, but I don't mind the previous one (perhaps tweaking it to update the size in the XML to the auto-aligned value, for consistency). I can post patches reverting this change and amending the older approach (which will not break existing guests). Andrea, what do you think? Thanks, DHB
Regards, Daniel
We don't need the auto-alignment now that the user is handling it by hand. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_domain.c | 51 ++++-------------------------------------- 1 file changed, 4 insertions(+), 47 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index dfe3c3b64b..dfbd8ec3b8 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7969,44 +7969,6 @@ qemuDomainGetMemoryModuleSizeAlignment(const virDomainDef *def, } -static int -qemuDomainNVDimmAlignSizePseries(virDomainDefPtr def, - virDomainMemoryDefPtr mem) -{ - /* For NVDIMMs in ppc64 in we want to align down the guest - * visible space, instead of align up, to avoid writing - * beyond the end of file by adding a potential 256MiB - * to the user specified size. - * - * The label-size is mandatory for ppc64 as well, meaning that - * the guest visible space will be target_size-label_size. - * - * Finally, target_size must include label_size. - * - * The above can be summed up as follows: - * - * target_size = AlignDown(target_size - label_size) + label_size - */ - unsigned long long ppc64AlignSize = qemuDomainGetMemorySizeAlignment(def); - unsigned long long guestArea = mem->size - mem->labelsize; - - /* Align down guest_area. 256MiB is the minimum size. Error - * out if target_size is smaller than 256MiB + label_size, - * since aligning it up will cause QEMU errors. */ - if (mem->size < (ppc64AlignSize + mem->labelsize)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("minimum target size for the NVDIMM " - "must be 256MB plus the label size")); - return -1; - } - - guestArea = (guestArea/ppc64AlignSize) * ppc64AlignSize; - mem->size = guestArea + mem->labelsize; - - return 0; -} - - int qemuDomainAlignMemorySizes(virDomainDefPtr def) { @@ -8053,11 +8015,8 @@ qemuDomainAlignMemorySizes(virDomainDefPtr def) /* Align memory module sizes */ for (i = 0; i < def->nmems; i++) { - if (def->mems[i]->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM && - ARCH_IS_PPC64(def->os.arch)) { - if (qemuDomainNVDimmAlignSizePseries(def, def->mems[i]) < 0) - return -1; - } else { + if (!(def->mems[i]->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM && + ARCH_IS_PPC64(def->os.arch))) { align = qemuDomainGetMemoryModuleSizeAlignment(def, def->mems[i]); def->mems[i]->size = VIR_ROUND_UP(def->mems[i]->size, align); } @@ -8090,10 +8049,8 @@ int qemuDomainMemoryDeviceAlignSize(virDomainDefPtr def, virDomainMemoryDefPtr mem) { - if (mem->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM && - ARCH_IS_PPC64(def->os.arch)) { - return qemuDomainNVDimmAlignSizePseries(def, mem); - } else { + if (!(mem->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM && + ARCH_IS_PPC64(def->os.arch))) { mem->size = VIR_ROUND_UP(mem->size, qemuDomainGetMemorySizeAlignment(def)); } -- 2.26.2
After the recent changes, this function is now always returning zero. Turn it to 'void' to relieve callers from checking it. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_domain.c | 4 +--- src/qemu/qemu_domain.h | 4 ++-- src/qemu/qemu_hotplug.c | 6 ++---- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index dfbd8ec3b8..5312c08be7 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8045,7 +8045,7 @@ qemuDomainAlignMemorySizes(virDomainDefPtr def) * inplace. Default rounding is now to 1 MiB (qemu requires rounding to page, * size so this should be safe). */ -int +void qemuDomainMemoryDeviceAlignSize(virDomainDefPtr def, virDomainMemoryDefPtr mem) { @@ -8054,8 +8054,6 @@ qemuDomainMemoryDeviceAlignSize(virDomainDefPtr def, mem->size = VIR_ROUND_UP(mem->size, qemuDomainGetMemorySizeAlignment(def)); } - - return 0; } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index cbd56861da..46cd66171c 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -761,8 +761,8 @@ bool qemuDomainHasBlockjob(virDomainObjPtr vm, bool copy_only) ATTRIBUTE_NONNULL(1); int qemuDomainAlignMemorySizes(virDomainDefPtr def); -int qemuDomainMemoryDeviceAlignSize(virDomainDefPtr def, - virDomainMemoryDefPtr mem); +void qemuDomainMemoryDeviceAlignSize(virDomainDefPtr def, + virDomainMemoryDefPtr mem); virDomainChrDefPtr qemuFindAgentConfig(virDomainDefPtr def); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 26912334d2..d4e2c2a86c 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2367,8 +2367,7 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, int id; int ret = -1; - if (qemuDomainMemoryDeviceAlignSize(vm->def, mem) < 0) - goto cleanup; + qemuDomainMemoryDeviceAlignSize(vm->def, mem); if (qemuDomainDefValidateMemoryHotplug(vm->def, priv->qemuCaps, mem) < 0) goto cleanup; @@ -5611,8 +5610,7 @@ qemuDomainDetachPrepMemory(virDomainObjPtr vm, virDomainMemoryDefPtr mem; int idx; - if (qemuDomainMemoryDeviceAlignSize(vm->def, match) < 0) - return -1; + qemuDomainMemoryDeviceAlignSize(vm->def, match); if ((idx = virDomainMemoryFindByDef(vm->def, match)) < 0) { virReportError(VIR_ERR_DEVICE_MISSING, -- 2.26.2
We do not auto-align down pSeries NVDIMMs anymore. This reverts commit 8f474ceea05aec349be19726e394a62e300efe77. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- docs/formatdomain.html.in | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 6b67a09bb3..fc429623f0 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -9413,10 +9413,8 @@ qemu-kvm -net nic,model=? /dev/null </p> <ol> <li>the minimum label size is 128KiB,</li> - <li>the remaining size (total-size - label-size), also called guest - area, will be aligned to 4KiB as default. For pSeries guests, the - guest area will be aligned down to 256MiB, and the minimum size - of the guest area must be at least 256MiB.</li> + <li>the remaining size (total-size - label-size) will be aligned + to 4KiB as default.</li> </ol> </dd> -- 2.26.2
Ping On 7/30/20 4:47 PM, Daniel Henrique Barboza wrote:
Hi,
In [1] Andrea proposed that we should *not* auto-align down the NVDIMM size for pSeries guests. Instead we should error out and provide users with a suggested aligned value. This patch implements it.
[1] https://www.redhat.com/archives/libvir-list/2020-July/msg01471.html
Daniel Henrique Barboza (5): qemu_domain.c: make qemuDomainGetMemorySizeAlignment() public qemu_validate.c: add pSeries NVDIMM size alignment validation qemu_domain.c: do not auto-align ppc64 NVDIMMs qemu_domain.c: change qemuDomainMemoryDeviceAlignSize() return type Revert "formatdomain.html.in: mention pSeries NVDIMM 'align down' mechanic"
docs/formatdomain.html.in | 6 +- src/qemu/qemu_domain.c | 57 ++----------------- src/qemu/qemu_domain.h | 7 ++- src/qemu/qemu_hotplug.c | 6 +- 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 +- 8 files changed, 55 insertions(+), 70 deletions(-)
On 7/30/20 9:47 PM, Daniel Henrique Barboza wrote:
Hi,
In [1] Andrea proposed that we should *not* auto-align down the NVDIMM size for pSeries guests. Instead we should error out and provide users with a suggested aligned value. This patch implements it.
[1] https://www.redhat.com/archives/libvir-list/2020-July/msg01471.html
Daniel Henrique Barboza (5): qemu_domain.c: make qemuDomainGetMemorySizeAlignment() public qemu_validate.c: add pSeries NVDIMM size alignment validation qemu_domain.c: do not auto-align ppc64 NVDIMMs qemu_domain.c: change qemuDomainMemoryDeviceAlignSize() return type Revert "formatdomain.html.in: mention pSeries NVDIMM 'align down' mechanic"
docs/formatdomain.html.in | 6 +- src/qemu/qemu_domain.c | 57 ++----------------- src/qemu/qemu_domain.h | 7 ++- src/qemu/qemu_hotplug.c | 6 +- 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 +- 8 files changed, 55 insertions(+), 70 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> and pushed. Sorry for delay. Do you think this deserves a NEWS.rst entry? Michal
On 8/24/20 1:44 PM, Michal Privoznik wrote:
On 7/30/20 9:47 PM, Daniel Henrique Barboza wrote:
Hi,
In [1] Andrea proposed that we should *not* auto-align down the NVDIMM size for pSeries guests. Instead we should error out and provide users with a suggested aligned value. This patch implements it.
[1] https://www.redhat.com/archives/libvir-list/2020-July/msg01471.html
Daniel Henrique Barboza (5): qemu_domain.c: make qemuDomainGetMemorySizeAlignment() public qemu_validate.c: add pSeries NVDIMM size alignment validation qemu_domain.c: do not auto-align ppc64 NVDIMMs qemu_domain.c: change qemuDomainMemoryDeviceAlignSize() return type Revert "formatdomain.html.in: mention pSeries NVDIMM 'align down' mechanic"
docs/formatdomain.html.in | 6 +- src/qemu/qemu_domain.c | 57 ++----------------- src/qemu/qemu_domain.h | 7 ++- src/qemu/qemu_hotplug.c | 6 +- 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 +- 8 files changed, 55 insertions(+), 70 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
and pushed. Sorry for delay. Do you think this deserves a NEWS.rst entry?
For 6.7.0? Yeah I think it would be good. Do you want me to post a patch updating it? If you're going to update NEWS.rst with other things feel free to add this info as well. Thanks, DHB
Michal
On 8/24/20 6:52 PM, Daniel Henrique Barboza wrote:
On 8/24/20 1:44 PM, Michal Privoznik wrote:
On 7/30/20 9:47 PM, Daniel Henrique Barboza wrote:
Hi,
In [1] Andrea proposed that we should *not* auto-align down the NVDIMM size for pSeries guests. Instead we should error out and provide users with a suggested aligned value. This patch implements it.
[1] https://www.redhat.com/archives/libvir-list/2020-July/msg01471.html
Daniel Henrique Barboza (5): qemu_domain.c: make qemuDomainGetMemorySizeAlignment() public qemu_validate.c: add pSeries NVDIMM size alignment validation qemu_domain.c: do not auto-align ppc64 NVDIMMs qemu_domain.c: change qemuDomainMemoryDeviceAlignSize() return type Revert "formatdomain.html.in: mention pSeries NVDIMM 'align down' mechanic"
docs/formatdomain.html.in | 6 +- src/qemu/qemu_domain.c | 57 ++----------------- src/qemu/qemu_domain.h | 7 ++- src/qemu/qemu_hotplug.c | 6 +- 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 +- 8 files changed, 55 insertions(+), 70 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
and pushed. Sorry for delay. Do you think this deserves a NEWS.rst entry?
For 6.7.0? Yeah I think it would be good. Do you want me to post a patch updating it? If you're going to update NEWS.rst with other things feel free to add this info as well.
Yeah, I plan to write NEWS.rst update tomorrow, but I could use some help :-) Michal
participants (3)
-
Daniel Henrique Barboza -
Daniel P. Berrangé -
Michal Privoznik