Re: [PATCH v2 3/5] conf, qemu: enable NVDIMM support for ppc64

On 11. 3. 2020 22:29, Daniel Henrique Barboza wrote: > Using the 'uuid' element for ppc64 NVDIMM memory added in the > previous patch, use it in qemuBuildMemoryDeviceStr() to pass > it over to QEMU. > > Another ppc64 restriction is the necessity of a mem->labelsize, > given than ppc64 only support label-area backed NVDIMMs. > > Finally, we don't want ppc64 NVDIMMs to align up due to the > high risk of going beyond the end of file with a 256MiB > increment that the user didn't predict. Align it down > instead. > > Signed-off-by: Daniel Henrique Barboza <danielhb413(a)gmail.com&gt; > --- > src/conf/domain_conf.c | 7 +++ > src/qemu/qemu_command.c | 7 +++ > src/qemu/qemu_domain.c | 47 +++++++++++++++++-- > .../memory-hotplug-nvdimm-ppc64.args | 32 +++++++++++++ > .../memory-hotplug-nvdimm-ppc64.xml | 5 +- > tests/qemuxml2argvtest.c | 4 ++ > .../memory-hotplug-nvdimm-ppc64.xml | 5 +- > 7 files changed, 102 insertions(+), 5 deletions(-) > create mode 100644 tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64.args > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index 3ae6c181c2..7f8018fed2 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -16582,6 +16582,13 @@ virDomainMemoryDefParseXML(virDomainXMLOptionPtr xmlopt, > if (virDomainMemoryTargetDefParseXML(node, ctxt, def) < 0) > goto error; > > + if (def->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM && > + ARCH_IS_PPC64(dom->os.arch) && def->labelsize == 0) { > + virReportError(VIR_ERR_XML_ERROR, "%s", > + _("label size is required for NVDIMM device")); > + goto error; > + } > + I think this should go into virDomainMemoryDefValidate() instead, because this is not strictly a parse error (e.g. like invalid UUID or missing element). The rest looks good. Can you please post a v3 of just this patch? I will handle the review and merge. Sorry for delayed review.

Michal