The alignment for the pSeries NVDIMM does not depend on runtime
constraints. This means that it can be done in device parse
time, instead of runtime, allowing the domain XML to reflect
what the auto-alignment would do when the domain starts.

This brings consistency between the NVDIMM size reported by the
domain XML and what the guest sees, without impacting existing
guests that are using an unaligned size - they'll work as usual,
but the domain XML will be updated with the actual size of the
NVDIMM.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 src/conf/domain_conf.c                        | 35 +++++++++++--------
 .../memory-hotplug-nvdimm-ppc64.xml           |  2 +-
 2 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 63434b9d3e..0fd16964c1 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -16965,6 +16965,25 @@ virDomainMemoryDefParseXML(virDomainXMLOptionPtr xmlopt,
     }
     VIR_FREE(tmp);
 
+    /* source */
+    if ((node = virXPathNode("./source", ctxt)) &&
+        virDomainMemorySourceDefParseXML(node, ctxt, def) < 0)
+        goto error;
+
+    /* target */
+    if (!(node = virXPathNode("./target", ctxt))) {
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("missing <target> element for <memory> device"));
+        goto error;
+    }
+
+    if (virDomainMemoryTargetDefParseXML(node, ctxt, def) < 0)
+        goto error;
+
+    /* The UUID calculation for pSeries NVDIMM can be done earlier,
+     * but we'll also need to handle the size alignment, which depends
+     * on virDomainMemoryTargetDefParseXML() already done. Let's do it
+     * all here. */
     if (def->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM &&
         ARCH_IS_PPC64(dom->os.arch)) {
         /* Extract nvdimm uuid or generate a new one */
@@ -16981,23 +17000,11 @@ virDomainMemoryDefParseXML(virDomainXMLOptionPtr xmlopt,
                            "%s", _("malformed uuid element"));
             goto error;
         }
-    }
-
-    /* source */
-    if ((node = virXPathNode("./source", ctxt)) &&
-        virDomainMemorySourceDefParseXML(node, ctxt, def) < 0)
-        goto error;
 
-    /* target */
-    if (!(node = virXPathNode("./target", ctxt))) {
-        virReportError(VIR_ERR_XML_ERROR, "%s",
-                       _("missing <target> element for <memory> device"));
-        goto error;
+        if (virDomainNVDimmAlignSizePseries(def) < 0)
+            goto error;
     }
 
-    if (virDomainMemoryTargetDefParseXML(node, ctxt, def) < 0)
-        goto error;
-
     if (virDomainDeviceInfoParseXML(xmlopt, memdevNode,
                                     &def->info, flags) < 0)
         goto error;
diff --git a/tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-ppc64.xml b/tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-ppc64.xml
index ae5a17d3c8..ecb1b83b4a 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'>524416</size>
         <node>0</node>
         <label>
           <size unit='KiB'>128</size>
-- 
2.26.2

On Mon, 2020-09-14 at 23:42 -0300, Daniel Henrique Barboza wrote:
> @@ -16981,23 +17000,11 @@ virDomainMemoryDefParseXML(virDomainXMLOptionPtr xmlopt,
> +        if (virDomainNVDimmAlignSizePseries(def) < 0)
> +            goto error;

The outcome of this change is good, and overall it doesn't negatively
impact existing guest; however, the ParseXML() function is not the
correct place to perform it, and it should happen in the PostParse()
callback instead.

If the restrictions on alignment are, as you claim in the commit
message for the patch before this one, not QEMU-specific, then a
reasonable home for the rounding logic could be
virDomainDefPostParseMemory(), but you could also leave it in the
QEMU driver by implementing a qemuDomainMemoryDefPostParse() function
that's called by the qemuDomainDeviceDefPostParse() dispatcher.

-- 
Andrea Bolognani / Red Hat / Virtualization