[libvirt] [PATCH 05/22] qemu: Refactor qemuTranslatePool source

Peter Krempa pkrempa at redhat.com
Mon Nov 25 16:11:49 UTC 2013


The function was a mess originally making decisions on volume type
instead of pool type. Refactor it so that it doesn't require a special
formatter function in qemu and use typecasted strings to avoid possible
future omittings.
---
 src/conf/domain_conf.h   |   1 +
 src/libvirt_private.syms |   1 +
 src/qemu/qemu_command.c  |  74 +++-------------------------
 src/qemu/qemu_conf.c     | 125 ++++++++++++++++++++++++++++++++---------------
 src/qemu/qemu_conf.h     |   2 +
 5 files changed, 96 insertions(+), 107 deletions(-)

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 4561ccc..a5ef2ca 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -686,6 +686,7 @@ struct _virDomainDiskSourcePoolDef {
     char *volume; /* volume name */
     int voltype; /* enum virStorageVolType, internal only */
     int pooltype; /* enum virStoragePoolType, internal only */
+    int actualtype; /* enum virDomainDiskType, internal only */
     int mode; /* enum virDomainDiskSourcePoolMode */
 };
 typedef virDomainDiskSourcePoolDef *virDomainDiskSourcePoolDefPtr;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 205fe56..aeb3568 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -693,6 +693,7 @@ virStoragePoolSourceFree;
 virStoragePoolSourceListFormat;
 virStoragePoolSourceListNewSource;
 virStoragePoolTypeFromString;
+virStoragePoolTypeToString;
 virStorageVolDefFindByKey;
 virStorageVolDefFindByName;
 virStorageVolDefFindByPath;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 8dc7e43..2a4390e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3775,67 +3775,6 @@ qemuBuildNBDString(virConnectPtr conn, virDomainDiskDefPtr disk, virBufferPtr op
     return 0;
 }

-static int
-qemuBuildVolumeString(virConnectPtr conn,
-                      virDomainDiskDefPtr disk,
-                      virBufferPtr opt)
-{
-    int ret = -1;
-
-    switch (disk->srcpool->voltype) {
-    case VIR_STORAGE_VOL_DIR:
-        if (!disk->readonly) {
-            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                           _("cannot create virtual FAT disks in read-write mode"));
-            goto cleanup;
-        }
-        if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)
-            virBufferEscape(opt, ',', ",", "file=fat:floppy:%s,", disk->src);
-        else
-            virBufferEscape(opt, ',', ",", "file=fat:%s,", disk->src);
-        break;
-    case VIR_STORAGE_VOL_BLOCK:
-        if (disk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN) {
-            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                           _("tray status 'open' is invalid for "
-                             "block type volume"));
-            goto cleanup;
-        }
-        if (disk->srcpool->pooltype == VIR_STORAGE_POOL_ISCSI) {
-            if (disk->srcpool->mode ==
-                VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT) {
-                if (qemuBuildISCSIString(conn, disk, opt) < 0)
-                    goto cleanup;
-                virBufferAddChar(opt, ',');
-            } else if (disk->srcpool->mode ==
-                       VIR_DOMAIN_DISK_SOURCE_POOL_MODE_HOST) {
-                virBufferEscape(opt, ',', ",", "file=%s,", disk->src);
-            }
-        } else {
-            virBufferEscape(opt, ',', ",", "file=%s,", disk->src);
-        }
-        break;
-    case VIR_STORAGE_VOL_FILE:
-        if (disk->auth.username) {
-            if (qemuBuildISCSIString(conn, disk, opt) < 0)
-                goto cleanup;
-            virBufferAddChar(opt, ',');
-        } else {
-            virBufferEscape(opt, ',', ",", "file=%s,", disk->src);
-        }
-        break;
-    case VIR_STORAGE_VOL_NETWORK:
-        /* Keep the compiler quiet, qemuTranslateDiskSourcePool already
-         * reported the unsupported error.
-         */
-        break;
-    }
-
-    ret = 0;
-
-cleanup:
-    return ret;
-}

 char *
 qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
@@ -3849,6 +3788,7 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
         virDomainDiskGeometryTransTypeToString(disk->geometry.trans);
     int idx = virDiskNameToIndex(disk->dst);
     int busid = -1, unitid = -1;
+    int actualType = qemuDiskGetActualType(disk);

     if (idx < 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -3932,12 +3872,13 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,

     /* disk->src is NULL when we use nbd disks */
     if ((disk->src ||
-        (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK &&
+        (actualType == VIR_DOMAIN_DISK_TYPE_NETWORK &&
          disk->protocol == VIR_DOMAIN_DISK_PROTOCOL_NBD)) &&
         !((disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY ||
            disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) &&
           disk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN)) {
-        if (disk->type == VIR_DOMAIN_DISK_TYPE_DIR) {
+
+        if (actualType == VIR_DOMAIN_DISK_TYPE_DIR) {
             /* QEMU only supports magic FAT format for now */
             if (disk->format > 0 && disk->format != VIR_STORAGE_FILE_FAT) {
                 virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -3955,7 +3896,7 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
                                 disk->src);
             else
                 virBufferEscape(&opt, ',', ",", "file=fat:%s,", disk->src);
-        } else if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) {
+        } else if (actualType == VIR_DOMAIN_DISK_TYPE_NETWORK) {
             switch (disk->protocol) {
             case VIR_DOMAIN_DISK_PROTOCOL_NBD:
                 if (qemuBuildNBDString(conn, disk, &opt) < 0)
@@ -4023,11 +3964,8 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
                 virBufferEscape(&opt, ',', ",", "%s,", disk->src);
                 break;
             }
-        } else if (disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME) {
-            if (qemuBuildVolumeString(conn, disk, &opt) < 0)
-                goto error;
         } else {
-            if ((disk->type == VIR_DOMAIN_DISK_TYPE_BLOCK) &&
+            if ((actualType == VIR_DOMAIN_DISK_TYPE_BLOCK) &&
                 (disk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN)) {
                 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                                _("tray status 'open' is invalid for "
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 5803850..e098c13 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1298,6 +1298,17 @@ cleanup:
     return ret;
 }

+
+int
+qemuDiskGetActualType(virDomainDiskDefPtr def)
+{
+    if (def->type == VIR_DOMAIN_DISK_TYPE_VOLUME)
+        return def->srcpool->actualtype;
+
+    return def->type;
+}
+
+
 int
 qemuTranslateDiskSourcePool(virConnectPtr conn,
                             virDomainDiskDefPtr def)
@@ -1319,70 +1330,106 @@ qemuTranslateDiskSourcePool(virConnectPtr conn,
     if (!(pool = virStoragePoolLookupByName(conn, def->srcpool->pool)))
         return -1;

+    if (virStoragePoolIsActive(pool) != 1) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("storage pool '%s' containing volume '%s' "
+                         "is not active"),
+                       def->srcpool->pool, def->srcpool->volume);
+        goto cleanup;
+    }
+
     if (!(vol = virStorageVolLookupByName(pool, def->srcpool->volume)))
         goto cleanup;

     if (virStorageVolGetInfo(vol, &info) < 0)
         goto cleanup;

-    if (def->startupPolicy &&
-        info.type != VIR_STORAGE_VOL_FILE) {
+    if (!(poolxml = virStoragePoolGetXMLDesc(pool, 0)))
+        goto cleanup;
+
+    if (!(pooldef = virStoragePoolDefParseString(poolxml)))
+        goto cleanup;
+
+    def->srcpool->pooltype = pooldef->type;
+    def->srcpool->voltype = info.type;
+
+    if (def->srcpool->mode && pooldef->type != VIR_STORAGE_POOL_ISCSI) {
         virReportError(VIR_ERR_XML_ERROR, "%s",
-                       _("'startupPolicy' is only valid for 'file' type volume"));
+                       _("disk source mode is only valid when "
+                         "storage pool is of iscsi type"));
         goto cleanup;
     }

-    switch (info.type) {
-    case VIR_STORAGE_VOL_FILE:
-    case VIR_STORAGE_VOL_DIR:
+    switch ((enum virStoragePoolType) pooldef->type) {
+    case VIR_STORAGE_POOL_DIR:
+    case VIR_STORAGE_POOL_FS:
+    case VIR_STORAGE_POOL_NETFS:
+    case VIR_STORAGE_POOL_LOGICAL:
+    case VIR_STORAGE_POOL_DISK:
+    case VIR_STORAGE_POOL_SCSI:
         if (!(def->src = virStorageVolGetPath(vol)))
             goto cleanup;
-        break;
-    case VIR_STORAGE_VOL_BLOCK:
-        if (!(poolxml = virStoragePoolGetXMLDesc(pool, 0)))
-            goto cleanup;
-
-        if (!(pooldef = virStoragePoolDefParseString(poolxml)))
-            goto cleanup;

-        if (def->srcpool->mode && pooldef->type != VIR_STORAGE_POOL_ISCSI) {
-            virReportError(VIR_ERR_XML_ERROR, "%s",
-                           _("disk source mode is only valid when "
-                             "storage pool is of iscsi type"));
+        switch (info.type) {
+        case VIR_STORAGE_VOL_FILE:
+            def->srcpool->actualtype = VIR_DOMAIN_DISK_TYPE_FILE;
+            break;
+
+        case VIR_STORAGE_VOL_DIR:
+            def->srcpool->actualtype = VIR_DOMAIN_DISK_TYPE_DIR;
+            break;
+
+        case VIR_STORAGE_VOL_BLOCK:
+            def->srcpool->actualtype = VIR_DOMAIN_DISK_TYPE_BLOCK;
+            break;
+
+        case VIR_STORAGE_VOL_NETWORK:
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("unexpected storage volume type '%s' "
+                             "for storage pool type '%s'"),
+                           virStorageVolTypeToString(info.type),
+                           virStoragePoolTypeToString(pooldef->type));
             goto cleanup;
         }

-        def->srcpool->pooltype = pooldef->type;
-        if (pooldef->type == VIR_STORAGE_POOL_ISCSI) {
-            /* Default to use the LUN's path on host */
-            if (!def->srcpool->mode)
-                def->srcpool->mode = VIR_DOMAIN_DISK_SOURCE_POOL_MODE_HOST;
-
-            if (def->srcpool->mode ==
-                VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT) {
-                if (qemuAddISCSIPoolSourceHost(def, pooldef) < 0)
-                    goto cleanup;
-            } else if (def->srcpool->mode ==
-                       VIR_DOMAIN_DISK_SOURCE_POOL_MODE_HOST) {
-                if (!(def->src = virStorageVolGetPath(vol)))
-                    goto cleanup;
-            }
+        break;
+
+    case VIR_STORAGE_POOL_ISCSI:
+       switch (def->srcpool->mode) {
+        case VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DEFAULT:
+        case VIR_DOMAIN_DISK_SOURCE_POOL_MODE_LAST:
+            def->srcpool->mode = VIR_DOMAIN_DISK_SOURCE_POOL_MODE_HOST;
+            def->srcpool->actualtype = VIR_DOMAIN_DISK_TYPE_BLOCK;
+            /* fallthrough */
+        case VIR_DOMAIN_DISK_SOURCE_POOL_MODE_HOST:
+            if (!(def->src = virStorageVolGetPath(vol)))
+                goto cleanup;
+            break;
+
+        case VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT:
+            def->srcpool->actualtype = VIR_DOMAIN_DISK_TYPE_NETWORK;
+            def->protocol = VIR_DOMAIN_DISK_PROTOCOL_ISCSI;

             if (qemuTranslateDiskSourcePoolAuth(def, pooldef) < 0)
                 goto cleanup;
-        } else {
-            if (!(def->src = virStorageVolGetPath(vol)))
+
+            if (qemuAddISCSIPoolSourceHost(def, pooldef) < 0)
                 goto cleanup;
+            break;
         }
-
         break;
-    case VIR_STORAGE_VOL_NETWORK:
-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                       _("Using network volume as disk source is not supported"));
+
+    case VIR_STORAGE_POOL_MPATH:
+    case VIR_STORAGE_POOL_RBD:
+    case VIR_STORAGE_POOL_SHEEPDOG:
+    case VIR_STORAGE_POOL_LAST:
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("using '%s' pools for backing 'volume' disks "
+                         "isn't yet supported"),
+                       virStoragePoolTypeToString(pooldef->type));
         goto cleanup;
     }

-    def->srcpool->voltype = info.type;
     ret = 0;
 cleanup:
     if (ret < 0)
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index f9ff7af..b9786b1 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -304,6 +304,8 @@ int qemuSetUnprivSGIO(virDomainDeviceDefPtr dev);
 int qemuDriverAllocateID(virQEMUDriverPtr driver);
 virDomainXMLOptionPtr virQEMUDriverCreateXMLConf(virQEMUDriverPtr driver);

+int qemuDiskGetActualType(virDomainDiskDefPtr def);
+
 int qemuTranslateDiskSourcePool(virConnectPtr conn,
                                 virDomainDiskDefPtr def);

-- 
1.8.4.3




More information about the libvir-list mailing list