Factor out a separate function to parse out the <model> element for
video devices.

Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
---
 src/conf/domain_conf.c | 95 ++++++++++++++++++++++++++----------------
 1 file changed, 59 insertions(+), 36 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index fda1c6caa6..2e153db94f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12536,17 +12536,13 @@ virDomainVideoDriverDefParseXML(xmlNodePtr node,
     return g_steal_pointer(&def);
 }
 
-static virDomainVideoDef *
-virDomainVideoDefParseXML(virDomainXMLOption *xmlopt,
-                          xmlNodePtr node,
-                          xmlXPathContextPtr ctxt,
-                          unsigned int flags)
+static int
+virDomainVideoModelDefParseXML(virDomainVideoDef *def,
+                               xmlNodePtr node,
+                               xmlXPathContextPtr ctxt)
 {
-    g_autoptr(virDomainVideoDef) def = NULL;
-    xmlNodePtr driver;
     xmlNodePtr accel_node;
     xmlNodePtr res_node;
-    VIR_XPATH_NODE_AUTORESTORE(ctxt)
     g_autofree char *type = NULL;
     g_autofree char *heads = NULL;
     g_autofree char *vram = NULL;
@@ -12555,81 +12551,108 @@ virDomainVideoDefParseXML(virDomainXMLOption *xmlopt,
     g_autofree char *vgamem = NULL;
     g_autofree char *primary = NULL;
 
-    if (!(def = virDomainVideoDefNew(xmlopt)))
-        return NULL;
-
+    VIR_XPATH_NODE_AUTORESTORE(ctxt);
     ctxt->node = node;
 
-    if ((primary = virXPathString("string(./model/@primary)", ctxt)) != NULL)
+    if ((primary = virXPathString("string(./@primary)", ctxt)) != NULL)
         ignore_value(virStringParseYesNo(primary, &def->primary));
 
-    if ((accel_node = virXPathNode("./model/acceleration", ctxt)) &&
+    if ((accel_node = virXPathNode("./acceleration", ctxt)) &&
         (def->accel = virDomainVideoAccelDefParseXML(accel_node)) == NULL)
-        return NULL;
+        return -1;
 
-    if ((res_node = virXPathNode("./model/resolution", ctxt)) &&
+    if ((res_node = virXPathNode("./resolution", ctxt)) &&
         (def->res = virDomainVideoResolutionDefParseXML(res_node)) == NULL)
-        return NULL;
+        return -1;
 
-    if ((driver = virXPathNode("./driver", ctxt))) {
-        if (virXMLPropEnum(driver, "name",
-                           virDomainVideoBackendTypeFromString,
-                           VIR_XML_PROP_NONZERO, &def->backend) < 0)
-            return NULL;
-        if (virDomainVirtioOptionsParseXML(driver, &def->virtio) < 0)
-            return NULL;
-    }
 
-    if ((type = virXPathString("string(./model/@type)", ctxt))) {
+
+    if ((type = virXPathString("string(./@type)", ctxt))) {
         if ((def->type = virDomainVideoTypeFromString(type)) < 0) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                            _("unknown video model '%s'"), type);
-            return NULL;
+            return -1;
         }
     } else {
         def->type = VIR_DOMAIN_VIDEO_TYPE_DEFAULT;
     }
 
-    if ((ram = virXPathString("string(./model/@ram)", ctxt))) {
+    if ((ram = virXPathString("string(./@ram)", ctxt))) {
         if (virStrToLong_uip(ram, NULL, 10, &def->ram) < 0) {
             virReportError(VIR_ERR_XML_ERROR,
                            _("cannot parse video ram '%s'"), ram);
-            return NULL;
+            return -1;
         }
     }
 
-    if ((vram = virXPathString("string(./model/@vram)", ctxt))) {
+    if ((vram = virXPathString("string(./@vram)", ctxt))) {
         if (virStrToLong_uip(vram, NULL, 10, &def->vram) < 0) {
             virReportError(VIR_ERR_XML_ERROR,
                            _("cannot parse video vram '%s'"), vram);
-            return NULL;
+            return -1;
         }
     }
 
-    if ((vram64 = virXPathString("string(./model/@vram64)", ctxt))) {
+    if ((vram64 = virXPathString("string(./@vram64)", ctxt))) {
         if (virStrToLong_uip(vram64, NULL, 10, &def->vram64) < 0) {
             virReportError(VIR_ERR_XML_ERROR,
                            _("cannot parse video vram64 '%s'"), vram64);
-            return NULL;
+            return -1;
         }
     }
 
-    if ((vgamem = virXPathString("string(./model/@vgamem)", ctxt))) {
+    if ((vgamem = virXPathString("string(./@vgamem)", ctxt))) {
         if (virStrToLong_uip(vgamem, NULL, 10, &def->vgamem) < 0) {
             virReportError(VIR_ERR_XML_ERROR,
                            _("cannot parse video vgamem '%s'"), vgamem);
-            return NULL;
+            return -1;
         }
     }
 
-    if ((heads = virXPathString("string(./model/@heads)", ctxt))) {
+    if ((heads = virXPathString("string(./@heads)", ctxt))) {
         if (virStrToLong_uip(heads, NULL, 10, &def->heads) < 0) {
             virReportError(VIR_ERR_INTERNAL_ERROR,
                            _("cannot parse video heads '%s'"), heads);
-            return NULL;
+            return -1;
         }
     }
 
+    return 0;
+}
+
+static virDomainVideoDef *
+virDomainVideoDefParseXML(virDomainXMLOption *xmlopt,
+                          xmlNodePtr node,
+                          xmlXPathContextPtr ctxt,
+                          unsigned int flags)
+{
+    g_autoptr(virDomainVideoDef) def = NULL;
+    xmlNodePtr driver;
+    xmlNodePtr model;
+
+    VIR_XPATH_NODE_AUTORESTORE(ctxt)
+
+    if (!(def = virDomainVideoDefNew(xmlopt)))
+        return NULL;
+
+    ctxt->node = node;
+
+    if ((model = virXPathNode("./model", ctxt))) {
+        if (virDomainVideoModelDefParseXML(def, model, ctxt) < 0)
+            return NULL;
+    } else {
+        def->type = VIR_DOMAIN_VIDEO_TYPE_DEFAULT;
+    }
+
+    if ((driver = virXPathNode("./driver", ctxt))) {
+        if (virXMLPropEnum(driver, "name",
+                           virDomainVideoBackendTypeFromString,
+                           VIR_XML_PROP_NONZERO, &def->backend) < 0)
+            return NULL;
+        if (virDomainVirtioOptionsParseXML(driver, &def->virtio) < 0)
+            return NULL;
+    }
+
     if (virDomainDeviceInfoParseXML(xmlopt, node, ctxt, &def->info, flags) < 0)
         return NULL;
 
-- 
2.38.1

On a Friday in 2022, Jonathon Jongsma wrote:
>Factor out a separate function to parse out the <model> element for
>video devices.
>
>Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
>---
> src/conf/domain_conf.c | 95 ++++++++++++++++++++++++++----------------
> 1 file changed, 59 insertions(+), 36 deletions(-)
>
>diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>index fda1c6caa6..2e153db94f 100644
>--- a/src/conf/domain_conf.c
>+++ b/src/conf/domain_conf.c
>@@ -12536,17 +12536,13 @@ virDomainVideoDriverDefParseXML(xmlNodePtr node,
>     return g_steal_pointer(&def);
> }
>
>-static virDomainVideoDef *
>-virDomainVideoDefParseXML(virDomainXMLOption *xmlopt,
>-                          xmlNodePtr node,
>-                          xmlXPathContextPtr ctxt,
>-                          unsigned int flags)
>+static int
>+virDomainVideoModelDefParseXML(virDomainVideoDef *def,
>+                               xmlNodePtr node,
>+                               xmlXPathContextPtr ctxt)
> {
>-    g_autoptr(virDomainVideoDef) def = NULL;
>-    xmlNodePtr driver;
>     xmlNodePtr accel_node;
>     xmlNodePtr res_node;
>-    VIR_XPATH_NODE_AUTORESTORE(ctxt)
>     g_autofree char *type = NULL;
>     g_autofree char *heads = NULL;
>     g_autofree char *vram = NULL;
>@@ -12555,81 +12551,108 @@ virDomainVideoDefParseXML(virDomainXMLOption *xmlopt,
>     g_autofree char *vgamem = NULL;
>     g_autofree char *primary = NULL;
>
>-    if (!(def = virDomainVideoDefNew(xmlopt)))
>-        return NULL;
>-
>+    VIR_XPATH_NODE_AUTORESTORE(ctxt);

There's no need to move this declaration/initialization and add a
semicolon to it.

>     ctxt->node = node;
>

[...]

>-    if ((type = virXPathString("string(./model/@type)", ctxt))) {
>+
>+    if ((type = virXPathString("string(./@type)", ctxt))) {
>         if ((def->type = virDomainVideoTypeFromString(type)) < 0) {
>             virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                            _("unknown video model '%s'"), type);
>-            return NULL;
>+            return -1;
>         }
>     } else {
>         def->type = VIR_DOMAIN_VIDEO_TYPE_DEFAULT;
>     }
>

This branch can be handled by virDomainVideoDefNew. Or by saying that
VIR_DOMAIN_VIDEO_TYPE_DEFAULT = 0.

To the rest:

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano