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@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 488f258d0027768a21c45a619023253a178347f1..cac60e3394eb7c659ae8799f81772d2fb0a5a154 100644 (file)
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -4012,15 +4012,45 @@ qemuValidateDomainDeviceDefHub(virDomainHubDefPtr hub,
 }
 
 
+static unsigned long long
+qemuValidateGetNVDIMMAlignedSizePseries(virDomainMemoryDefPtr mem,
+                                        const virDomainDef *def)
+{
+    unsigned long long ppc64AlignSize = qemuDomainGetMemorySizeAlignment(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 +4168,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 eff80dcf80f0d3f30260f0a45bf097f43dec2620..58e3f9e1613020cd2abc063aebcb6f8e89ccb6c7 100644 (file)
--- 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 ae5a17d3c8d731e394158e61174b778cc52b23a2..10c146e8cfce4d8db799000ccb89f806fbe5ef9b 100644 (file)
--- 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 ae5a17d3c8d731e394158e61174b778cc52b23a2..10c146e8cfce4d8db799000ccb89f806fbe5ef9b 100644 (file)
--- 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>