xen_xm: Split the per-disk logic from xenParseXMDisk()

xenParseXMDisk() does a lot of stuff and, in order to make things
cleaner, let's split it in two new functions:
- xenParseXMDisk(): it's a new function that keeps the old name. It's
responsible for the whole per-disk logic from the old xenParseXMDisk();
- xenParseXMDiskList(): it's basically the old xenParseXMDisk(), but
now it just iterates over the list of disks, calling xenParseXMDisk()
per each disk.

This patch is basically preparing the ground for the future when
typesafe virConf acessors will be used.
Signed-off-by: NFabiano Fidêncio <fabiano@fidencio.org>
Reviewed-by: NJán Tomko <jtomko@redhat.com>
Signed-off-by: NJán Tomko <jtomko@redhat.com>

src/xenconfig/xen_xm.c

@@ -107,179 +107,187 @@ xenParseXMOS(virConfPtr conf, virDomainDefPtr def)
 }
 
 
-static int
-xenParseXMDisk(virConfPtr conf, virDomainDefPtr def)
+static virDomainDiskDefPtr
+xenParseXMDisk(char *entry, int hvm)
 {
     virDomainDiskDefPtr disk = NULL;
-    int hvm = def->os.type == VIR_DOMAIN_OSTYPE_HVM;
-    virConfValuePtr list = virConfGetValue(conf, "disk");
+    char *head;
+    char *offset;
+    char *tmp;
+    const char *src;
 
-    if (list && list->type == VIR_CONF_LIST) {
-        list = list->list;
-        while (list) {
-            char *head;
-            char *offset;
-            char *tmp;
-            const char *src;
+    if (!(disk = virDomainDiskDefNew(NULL)))
+        return NULL;
 
-            if ((list->type != VIR_CONF_STRING) || (list->str == NULL))
-                goto skipdisk;
+    head = entry;
+    /*
+     * Disks have 3 components, SOURCE,DEST-DEVICE,MODE
+     * eg, phy:/dev/HostVG/XenGuest1,xvda,w
+     * The SOURCE is usually prefixed with a driver type,
+     * and optionally driver sub-type
+     * The DEST-DEVICE is optionally post-fixed with disk type
+     */
+
+    /* Extract the source file path*/
+    if (!(offset = strchr(head, ',')))
+        goto error;
+
+    if (offset == head) {
+        /* No source file given, eg CDROM with no media */
+        ignore_value(virDomainDiskSetSource(disk, NULL));
+    } else {
+        if (VIR_STRNDUP(tmp, head, offset - head) < 0)
+            goto error;
 
-            head = list->str;
-            if (!(disk = virDomainDiskDefNew(NULL)))
-                return -1;
+        if (virDomainDiskSetSource(disk, tmp) < 0) {
+            VIR_FREE(tmp);
+            goto error;
+        }
+        VIR_FREE(tmp);
+    }
 
-            /*
-             * Disks have 3 components, SOURCE,DEST-DEVICE,MODE
-             * eg, phy:/dev/HostVG/XenGuest1,xvda,w
-             * The SOURCE is usually prefixed with a driver type,
-             * and optionally driver sub-type
-             * The DEST-DEVICE is optionally post-fixed with disk type
-             */
-
-            /* Extract the source file path*/
-            if (!(offset = strchr(head, ',')))
-                goto skipdisk;
-
-            if (offset == head) {
-                /* No source file given, eg CDROM with no media */
-                ignore_value(virDomainDiskSetSource(disk, NULL));
-            } else {
-                if (VIR_STRNDUP(tmp, head, offset - head) < 0)
-                    goto cleanup;
-
-                if (virDomainDiskSetSource(disk, tmp) < 0) {
-                    VIR_FREE(tmp);
-                    goto cleanup;
-                }
+    head = offset + 1;
+    /* Remove legacy ioemu: junk */
+    if (STRPREFIX(head, "ioemu:"))
+        head = head + 6;
+
+    /* Extract the dest device name */
+    if (!(offset = strchr(head, ',')))
+        goto error;
+
+    if (VIR_ALLOC_N(disk->dst, (offset - head) + 1) < 0)
+        goto error;
+
+    if (virStrncpy(disk->dst, head, offset - head,
+                   (offset - head) + 1) == NULL) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Dest file %s too big for destination"), head);
+        goto error;
+    }
+
+    head = offset + 1;
+    /* Extract source driver type */
+    src = virDomainDiskGetSource(disk);
+    if (src) {
+        size_t len;
+        /* The main type  phy:, file:, tap: ... */
+        if ((tmp = strchr(src, ':')) != NULL) {
+            len = tmp - src;
+            if (VIR_STRNDUP(tmp, src, len) < 0)
+                goto error;
+
+            if (virDomainDiskSetDriver(disk, tmp) < 0) {
                 VIR_FREE(tmp);
+                goto error;
             }
+            VIR_FREE(tmp);
 
-            head = offset + 1;
-            /* Remove legacy ioemu: junk */
-            if (STRPREFIX(head, "ioemu:"))
-                head = head + 6;
+            /* Strip the prefix we found off the source file name */
+            if (virDomainDiskSetSource(disk, src + len + 1) < 0)
+                goto error;
 
-            /* Extract the dest device name */
-            if (!(offset = strchr(head, ',')))
-                goto skipdisk;
+            src = virDomainDiskGetSource(disk);
+        }
 
-            if (VIR_ALLOC_N(disk->dst, (offset - head) + 1) < 0)
-                goto cleanup;
+        /* And the sub-type for tap:XXX: type */
+        if (STREQ_NULLABLE(virDomainDiskGetDriver(disk), "tap") ||
+            STREQ_NULLABLE(virDomainDiskGetDriver(disk), "tap2")) {
+            char *driverType;
 
-            if (virStrncpy(disk->dst, head, offset - head,
-                           (offset - head) + 1) == NULL) {
+            if (!(tmp = strchr(src, ':')))
+                goto error;
+            len = tmp - src;
+
+            if (VIR_STRNDUP(driverType, src, len) < 0)
+                goto error;
+
+            if (STREQ(driverType, "aio"))
+                virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_RAW);
+            else
+                virDomainDiskSetFormat(disk,
+                                       virStorageFileFormatTypeFromString(driverType));
+            VIR_FREE(driverType);
+            if (virDomainDiskGetFormat(disk) <= 0) {
                 virReportError(VIR_ERR_INTERNAL_ERROR,
-                               _("Dest file %s too big for destination"), head);
-                goto cleanup;
+                               _("Unknown driver type %s"),
+                               src);
+                goto error;
             }
 
-            head = offset + 1;
-            /* Extract source driver type */
+            /* Strip the prefix we found off the source file name */
+            if (virDomainDiskSetSource(disk, src + len + 1) < 0)
+                goto error;
             src = virDomainDiskGetSource(disk);
-            if (src) {
-                size_t len;
-                /* The main type  phy:, file:, tap: ... */
-                if ((tmp = strchr(src, ':')) != NULL) {
-                    len = tmp - src;
-                    if (VIR_STRNDUP(tmp, src, len) < 0)
-                        goto cleanup;
-
-                    if (virDomainDiskSetDriver(disk, tmp) < 0) {
-                        VIR_FREE(tmp);
-                        goto cleanup;
-                    }
-                    VIR_FREE(tmp);
-
-                    /* Strip the prefix we found off the source file name */
-                    if (virDomainDiskSetSource(disk, src + len + 1) < 0)
-                        goto cleanup;
-
-                    src = virDomainDiskGetSource(disk);
-                }
+        }
+    }
 
-                /* And the sub-type for tap:XXX: type */
-                if (STREQ_NULLABLE(virDomainDiskGetDriver(disk), "tap") ||
-                    STREQ_NULLABLE(virDomainDiskGetDriver(disk), "tap2")) {
-                    char *driverType;
-
-                    if (!(tmp = strchr(src, ':')))
-                        goto skipdisk;
-                    len = tmp - src;
-
-                    if (VIR_STRNDUP(driverType, src, len) < 0)
-                        goto cleanup;
-
-                    if (STREQ(driverType, "aio"))
-                        virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_RAW);
-                    else
-                        virDomainDiskSetFormat(disk,
-                                               virStorageFileFormatTypeFromString(driverType));
-                    VIR_FREE(driverType);
-                    if (virDomainDiskGetFormat(disk) <= 0) {
-                        virReportError(VIR_ERR_INTERNAL_ERROR,
-                                       _("Unknown driver type %s"),
-                                       src);
-                        goto cleanup;
-                    }
-
-                    /* Strip the prefix we found off the source file name */
-                    if (virDomainDiskSetSource(disk, src + len + 1) < 0)
-                        goto cleanup;
-                    src = virDomainDiskGetSource(disk);
-                }
-            }
+    /* No source, or driver name, so fix to phy: */
+    if (!virDomainDiskGetDriver(disk) &&
+        virDomainDiskSetDriver(disk, "phy") < 0)
+        goto error;
+
+    /* phy: type indicates a block device */
+    virDomainDiskSetType(disk,
+                         STREQ(virDomainDiskGetDriver(disk), "phy") ?
+                         VIR_STORAGE_TYPE_BLOCK :
+                         VIR_STORAGE_TYPE_FILE);
+
+    /* Check for a :cdrom/:disk postfix */
+    disk->device = VIR_DOMAIN_DISK_DEVICE_DISK;
+    if ((tmp = strchr(disk->dst, ':')) != NULL) {
+        if (STREQ(tmp, ":cdrom"))
+            disk->device = VIR_DOMAIN_DISK_DEVICE_CDROM;
+        tmp[0] = '\0';
+    }
 
-            /* No source, or driver name, so fix to phy: */
-            if (!virDomainDiskGetDriver(disk) &&
-                virDomainDiskSetDriver(disk, "phy") < 0)
-                goto cleanup;
+    if (STRPREFIX(disk->dst, "xvd") || !hvm)
+        disk->bus = VIR_DOMAIN_DISK_BUS_XEN;
+    else if (STRPREFIX(disk->dst, "sd"))
+        disk->bus = VIR_DOMAIN_DISK_BUS_SCSI;
+    else
+        disk->bus = VIR_DOMAIN_DISK_BUS_IDE;
 
-            /* phy: type indicates a block device */
-            virDomainDiskSetType(disk,
-                                 STREQ(virDomainDiskGetDriver(disk), "phy") ?
-                                 VIR_STORAGE_TYPE_BLOCK :
-                                 VIR_STORAGE_TYPE_FILE);
-
-            /* Check for a :cdrom/:disk postfix */
-            disk->device = VIR_DOMAIN_DISK_DEVICE_DISK;
-            if ((tmp = strchr(disk->dst, ':')) != NULL) {
-                if (STREQ(tmp, ":cdrom"))
-                    disk->device = VIR_DOMAIN_DISK_DEVICE_CDROM;
-                tmp[0] = '\0';
-            }
+    if (STREQ(head, "r") || STREQ(head, "ro"))
+        disk->src->readonly = true;
+    else if (STREQ(head, "w!") || STREQ(head, "!"))
+        disk->src->shared = true;
 
-            if (STRPREFIX(disk->dst, "xvd") || !hvm) {
-                disk->bus = VIR_DOMAIN_DISK_BUS_XEN;
-            } else if (STRPREFIX(disk->dst, "sd")) {
-                disk->bus = VIR_DOMAIN_DISK_BUS_SCSI;
-            } else {
-                disk->bus = VIR_DOMAIN_DISK_BUS_IDE;
-            }
+    return disk;
 
-            if (STREQ(head, "r") ||
-                STREQ(head, "ro"))
-                disk->src->readonly = true;
-            else if ((STREQ(head, "w!")) ||
-                     (STREQ(head, "!")))
-                disk->src->shared = true;
+ error:
+    virDomainDiskDefFree(disk);
+    return NULL;
+}
 
-            /* Maintain list in sorted order according to target device name */
-            if (VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk) < 0)
-                goto cleanup;
 
-            skipdisk:
-            list = list->next;
-            virDomainDiskDefFree(disk);
-            disk = NULL;
-        }
+static int
+xenParseXMDiskList(virConfPtr conf, virDomainDefPtr def)
+{
+    int hvm = def->os.type == VIR_DOMAIN_OSTYPE_HVM;
+    virConfValuePtr list = virConfGetValue(conf, "disk");
+
+    if (!list || list->type != VIR_CONF_LIST)
+        return 0;
+
+    for (list = list->list; list; list = list->next) {
+        virDomainDiskDefPtr disk;
+        int rc;
+
+        if ((list->type != VIR_CONF_STRING) || (list->str == NULL))
+            continue;
+
+        if (!(disk = xenParseXMDisk(list->str, hvm)))
+            continue;
+
+        /* Maintain list in sorted order according to target device name */
+        rc = VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk);
+        virDomainDiskDefFree(disk);
+
+        if (rc < 0)
+            return -1;
     }
 
     return 0;
-
- cleanup:
-    virDomainDiskDefFree(disk);
-    return -1;
 }
 
 
@@ -457,7 +465,7 @@ xenParseXM(virConfPtr conf,
     if (xenParseXMOS(conf, def) < 0)
          goto cleanup;
 
-    if (xenParseXMDisk(conf, def) < 0)
+    if (xenParseXMDiskList(conf, def) < 0)
          goto cleanup;
 
     if (xenParseXMInputDevs(conf, def) < 0)