conf: domain: Split out parsing of source data from <disk> XML parser

Extract all code related to parsing data which ends up in the 'src'
member of a virDomainDiskDef.

This allows to use the new function directly in
virDomainDiskDefParseSource and removes the use of the
VIR_DOMAIN_DEF_PARSE_DISK_SOURCE parser flag.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

src/conf/domain_conf.c

@@ -9341,6 +9341,86 @@ virDomainDiskDefParsePrivateData(xmlXPathContextPtr ctxt,
 }
 
 
+static virStorageSource *
+virDomainDiskDefParseSourceXML(virDomainXMLOption *xmlopt,
+                               xmlNodePtr node,
+                               xmlXPathContextPtr ctxt,
+                               unsigned int flags)
+{
+    g_autoptr(virStorageSource) src = virStorageSourceNew();
+    VIR_XPATH_NODE_AUTORESTORE(ctxt)
+    g_autofree char *type = NULL;
+    xmlNodePtr tmp;
+
+    ctxt->node = node;
+
+    src->type = VIR_STORAGE_TYPE_FILE;
+
+    if ((type = virXMLPropString(node, "type")) &&
+        (src->type = virStorageTypeFromString(type)) <= 0) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("unknown disk type '%s'"), type);
+        return NULL;
+    }
+
+    if ((tmp = virXPathNode("./source[1]", ctxt))) {
+        if (virDomainStorageSourceParse(tmp, ctxt, src, flags, xmlopt) < 0)
+            return NULL;
+
+        if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)) {
+            g_autofree char *sourceindex = NULL;
+
+            if ((sourceindex = virXMLPropString(tmp, "index")) &&
+                virStrToLong_uip(sourceindex, NULL, 10, &src->id) < 0) {
+                virReportError(VIR_ERR_XML_ERROR,
+                               _("invalid disk index '%s'"), sourceindex);
+                return NULL;
+            }
+        }
+    } else {
+        /* Reset src->type in case when 'source' was not present */
+        src->type = VIR_STORAGE_TYPE_FILE;
+    }
+
+    if (virXPathNode("./readonly[1]", ctxt))
+        src->readonly = true;
+
+    if (virXPathNode("./shareable[1]", ctxt))
+        src->shared = true;
+
+    if ((tmp = virXPathNode("./auth", ctxt))) {
+        /* If we've already parsed <source> and found an <auth> child,
+         * then generate an error to avoid ambiguity */
+        if (src->auth) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                           _("an <auth> definition already found for disk source"));
+            return NULL;
+        }
+
+        if (!(src->auth = virStorageAuthDefParse(tmp, ctxt)))
+            return NULL;
+    }
+
+    if ((tmp = virXPathNode("./encryption", ctxt))) {
+        /* If we've already parsed <source> and found an <encryption> child,
+         * then generate an error to avoid ambiguity */
+        if (src->encryption) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                           _("an <encryption> definition already found for disk source"));
+            return NULL;
+        }
+
+        if (!(src->encryption = virStorageEncryptionParseNode(tmp, ctxt)))
+            return NULL;
+    }
+
+    if (virDomainDiskBackingStoreParse(ctxt, src, flags, xmlopt) < 0)
+        return NULL;
+
+    return g_steal_pointer(&src);
+}
+
+
 static virDomainDiskDef *
 virDomainDiskDefParseXML(virDomainXMLOption *xmlopt,
                          xmlNodePtr node,
@@ -9351,8 +9431,6 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt,
     xmlNodePtr cur;
     VIR_XPATH_NODE_AUTORESTORE(ctxt)
     bool source = false;
-    g_autoptr(virStorageEncryption) encryption = NULL;
-    g_autoptr(virStorageAuthDef) authdef = NULL;
     g_autofree char *tmp = NULL;
     g_autofree char *snapshot = NULL;
     g_autofree char *rawio = NULL;
@@ -9368,24 +9446,19 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt,
     g_autofree char *product = NULL;
     g_autofree char *domain_name = NULL;
     g_autofree char *rotation_rate = NULL;
+    g_autoptr(virStorageSource) src = NULL;
 
-    if (!(def = virDomainDiskDefNew(xmlopt)))
+    if (!(src = virDomainDiskDefParseSourceXML(xmlopt, node, ctxt, flags)))
+        return NULL;
+
+    if (!(def = virDomainDiskDefNewSource(xmlopt, &src)))
         return NULL;
 
     ctxt->node = node;
 
     /* defaults */
-    def->src->type = VIR_STORAGE_TYPE_FILE;
     def->device = VIR_DOMAIN_DISK_DEVICE_DISK;
 
-    if ((tmp = virXMLPropString(node, "type")) &&
-        (def->src->type = virStorageTypeFromString(tmp)) <= 0) {
-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                       _("unknown disk type '%s'"), tmp);
-        return NULL;
-    }
-    VIR_FREE(tmp);
-
     if ((tmp = virXMLPropString(node, "device")) &&
         (def->device = virDomainDiskDeviceTypeFromString(tmp)) < 0) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -9412,9 +9485,6 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt,
             continue;
 
         if (!source && virXMLNodeNameEqual(cur, "source")) {
-            if (virDomainStorageSourceParse(cur, ctxt, def->src, flags, xmlopt) < 0)
-                return NULL;
-
             source = true;
 
             if (virXMLPropEnum(cur, "startupPolicy",
@@ -9423,13 +9493,6 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt,
                                &def->startupPolicy) < 0)
                 return NULL;
 
-            if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) &&
-                (tmp = virXMLPropString(cur, "index")) &&
-                virStrToLong_uip(tmp, NULL, 10, &def->src->id) < 0) {
-                virReportError(VIR_ERR_XML_ERROR, _("invalid disk index '%s'"), tmp);
-                return NULL;
-            }
-            VIR_FREE(tmp);
         } else if (!target &&
                    virXMLNodeNameEqual(cur, "target")) {
             target = virXMLPropString(cur, "dev");
@@ -9484,27 +9547,15 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt,
                    !(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)) {
             if (virDomainDiskDefMirrorParse(def, cur, ctxt, flags, xmlopt) < 0)
                 return NULL;
-        } else if (!authdef &&
+        } else if (virXMLNodeNameEqual(cur, "auth")) {
-                   virXMLNodeNameEqual(cur, "auth")) {
-            if (!(authdef = virStorageAuthDefParse(cur, ctxt)))
-                return NULL;
             def->diskElementAuth = true;
         } else if (virXMLNodeNameEqual(cur, "iotune")) {
             if (virDomainDiskDefIotuneParse(def, ctxt) < 0)
                 return NULL;
-        } else if (virXMLNodeNameEqual(cur, "readonly")) {
-            def->src->readonly = true;
-        } else if (virXMLNodeNameEqual(cur, "shareable")) {
-            def->src->shared = true;
         } else if (virXMLNodeNameEqual(cur, "transient")) {
             def->transient = true;
-        } else if (!encryption &&
+        } else if (virXMLNodeNameEqual(cur, "encryption")) {
-                   virXMLNodeNameEqual(cur, "encryption")) {
-            if (!(encryption = virStorageEncryptionParseNode(cur, ctxt)))
-                return NULL;
-
             def->diskElementEnc = true;
-
         } else if (!serial &&
                    virXMLNodeNameEqual(cur, "serial")) {
             if (!(serial = virXMLNodeContentString(cur)))
@@ -9548,10 +9599,6 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt,
         }
     }
 
-    /* Reset def->src->type in case when 'source' was not present */
-    if (!source)
-        def->src->type = VIR_STORAGE_TYPE_FILE;
-
     /* Only CDROM and Floppy devices are allowed missing source path
      * to indicate no media present. LUN is for raw access CD-ROMs
      * that are not attached to a physical device presently */
@@ -9676,40 +9723,12 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt,
     }
 
     def->dst = g_steal_pointer(&target);
-    if (authdef) {
-            /* If we've already parsed <source> and found an <auth> child,
-             * then generate an error to avoid ambiguity */
-            if (def->src->auth) {
-                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                               _("an <auth> definition already found for "
-                                 "disk source"));
-                return NULL;
-            }
-
-        def->src->auth = g_steal_pointer(&authdef);
-    }
-
-    if (encryption) {
-            /* If we've already parsed <source> and found an <encryption> child,
-             * then generate an error to avoid ambiguity */
-            if (def->src->encryption) {
-                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                               _("an <encryption> definition already found for "
-                                 "disk source"));
-                return NULL;
-            }
-
-        def->src->encryption = g_steal_pointer(&encryption);
-    }
     def->domain_name = g_steal_pointer(&domain_name);
     def->serial = g_steal_pointer(&serial);
     def->wwn = g_steal_pointer(&wwn);
     def->vendor = g_steal_pointer(&vendor);
     def->product = g_steal_pointer(&product);
 
-    if (virDomainDiskBackingStoreParse(ctxt, def->src, flags, xmlopt) < 0)
-        return NULL;
-
     if (flags & VIR_DOMAIN_DEF_PARSE_STATUS &&
         virDomainDiskDefParsePrivateData(ctxt, def, xmlopt) < 0)
         return NULL;
@@ -9721,22 +9740,6 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt,
 }
 
 
-static virStorageSource *
-virDomainDiskDefParseSourceXML(virDomainXMLOption *xmlopt,
-                               xmlNodePtr node,
-                               xmlXPathContextPtr ctxt,
-                               unsigned int flags)
-{
-    g_autoptr(virDomainDiskDef) diskdef = NULL;
-
-    if (!(diskdef = virDomainDiskDefParseXML(xmlopt, node, ctxt,
-                                             flags | VIR_DOMAIN_DEF_PARSE_DISK_SOURCE)))
-        return NULL;
-
-    return g_steal_pointer(&diskdef->src);
-}
-
-
 /**
  * virDomainParseMemory:
  * @xpath: XPath to memory amount
@@ -16484,11 +16487,23 @@ virDomainDiskDefParseSource(const char *xmlStr,
 {
     g_autoptr(xmlDoc) xml = NULL;
     g_autoptr(xmlXPathContext) ctxt = NULL;
+    g_autoptr(virStorageSource) src = NULL;
 
     if (!(xml = virXMLParseStringCtxtRoot(xmlStr, _("(disk_definition)"), "disk", &ctxt)))
         return NULL;
 
-    return virDomainDiskDefParseSourceXML(xmlopt, ctxt->node, ctxt, flags);
+    if (!(src = virDomainDiskDefParseSourceXML(xmlopt, ctxt->node, ctxt, flags)))
+        return NULL;
+
+    if (virStorageSourceIsEmpty(src)) {
+        virReportError(VIR_ERR_NO_SOURCE, NULL);
+        return NULL;
+    }
+
+    if (virDomainDiskDefParseValidateSource(src) < 0)
+        return NULL;
+
+    return g_steal_pointer(&src);
 }