2:47 p.m.
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
2:48 p.m.
New subject: [PATCH 1/5] qemu_domain.c: make qemuDomainGetMemorySizeAlignment() public
Next patch will use it outside of qemu_domain.c.
Signed-off-by: Daniel Henrique Barboza <danielhb413(a)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
11:44 a.m.
New subject: [PATCH 1/5] qemu_domain.c: make qemuDomainGetMemorySizeAlignment() public
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(a)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
2:48 p.m.
New subject: [PATCH 2/5] qemu_validate.c: add pSeries NVDIMM size alignment validation
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(a)redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413(a)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
11:44 a.m.
New subject: [PATCH 2/5] qemu_validate.c: add pSeries NVDIMM size alignment validation
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(a)redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413(a)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
11:50 a.m.
New subject: [PATCH 2/5] qemu_validate.c: add pSeries NVDIMM size alignment validation
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(a)redhat.com>
> Signed-off-by: Daniel Henrique Barboza <danielhb413(a)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
9:59 a.m.
New subject: [PATCH 2/5] qemu_validate.c: add pSeries NVDIMM size alignment validation
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 :|
12:37 p.m.
New subject: [PATCH 2/5] qemu_validate.c: add pSeries NVDIMM size alignment validation
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
2:48 p.m.
New subject: [PATCH 3/5] qemu_domain.c: do not auto-align ppc64 NVDIMMs
We don't need the auto-alignment now that the user is handling
it by hand.
Signed-off-by: Daniel Henrique Barboza <danielhb413(a)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
2:48 p.m.
New subject: [PATCH 4/5] qemu_domain.c: change qemuDomainMemoryDeviceAlignSize() return type
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(a)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
2:48 p.m.
New subject: [PATCH 5/5] Revert "formatdomain.html.in: mention pSeries NVDIMM 'align down' mechanic"
We do not auto-align down pSeries NVDIMMs anymore.
This reverts commit 8f474ceea05aec349be19726e394a62e300efe77.
Signed-off-by: Daniel Henrique Barboza <danielhb413(a)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
5:54 a.m.
New subject: [PATCH 0/5] remove NVDIMM auto-alignment for pSeries guests
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(-)
11:44 a.m.
New subject: [PATCH 0/5] remove NVDIMM auto-alignment for pSeries guests
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(a)redhat.com>
and pushed. Sorry for delay. Do you think this deserves a NEWS.rst entry?
Michal
11:52 a.m.
New subject: [PATCH 0/5] remove NVDIMM auto-alignment for pSeries guests
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(a)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
12:22 p.m.
New subject: [PATCH 0/5] remove NVDIMM auto-alignment for pSeries guests
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(a)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
1564
days inactive
1606
days old
14 comments
3 participants
participants (3)
-
Daniel Henrique Barboza -
Daniel P. Berrangé -
Michal Privoznik