On Mon, Jan 18, 2021 at 15:15:54 +0100, Michal Privoznik wrote:
> It's better to fill in missing values in post parse callbacks
> than during parsing.
>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
> src/conf/domain_conf.c | 35 ++++++++++++++++++++++-------------
> 1 file changed, 22 insertions(+), 13 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index a2ddfcf947..4f0798de45 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
[...]
> @@ -5345,6 +5346,19 @@ virDomainMemoryDefPostParse(virDomainMemoryDefPtr mem)
> break;
>
> case VIR_DOMAIN_MEMORY_MODEL_NVDIMM:
> + /* If no NVDIMM UUID was provided in XML, generate one. */
> + if (ARCH_IS_PPC64(def->os.arch) &&
> + !mem->uuid) {
> +
> + mem->uuid = g_new0(unsigned char, VIR_UUID_BUFLEN);
> + if (virUUIDGenerate(mem->uuid) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + "%s", _("Failed to generate UUID"));
> + return -1;
> + }
> + }
You can also reject if an UUID is present but the architecture isn't
PPC64 here, rather than ...
> + break;
> +
> case VIR_DOMAIN_MEMORY_MODEL_DIMM:
> case VIR_DOMAIN_MEMORY_MODEL_NONE:
> case VIR_DOMAIN_MEMORY_MODEL_LAST:
[...]
> @@ -15526,24 +15540,19 @@ virDomainMemoryDefParseXML(virDomainXMLOptionPtr xmlopt,
> }
> VIR_FREE(tmp);
>
> + /* Extract NVDIMM UUID. */
> if (def->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM &&
> - ARCH_IS_PPC64(dom->os.arch)) {
> - /* Extract nvdimm uuid or generate a new one */
> - tmp = virXPathString("string(./uuid[1])", ctxt);
> -
> + ARCH_IS_PPC64(dom->os.arch) &&
... keeping it in the parser, where it doesn't make much sense.
> + (tmp = virXPathString("string(./uuid[1])", ctxt))) {
> def->uuid = g_new0(unsigned char, VIR_UUID_BUFLEN);
> - if (!tmp) {
> - if (virUUIDGenerate(def->uuid) < 0) {
> - virReportError(VIR_ERR_INTERNAL_ERROR,
> - "%s", _("Failed to generate UUID"));
> - goto error;
> - }
> - } else if (virUUIDParse(tmp, def->uuid) < 0) {
> +
Reviewed-by: Peter Krempa <pkrempa@xxxxxxxxxx>
