storage: make disk source pool translation generic

Currently, qemu driver uses qemuTranslateDiskSourcePool()
to translate disk volume information. This function is
general enough and could be used for other drivers as well,
so move it to conf/domain_conf.c along with its helpers.

 - qemuTranslateDiskSourcePool: move to storage/storage_driver.c
   and rename to virStorageTranslateDiskSourcePool,
 - qemuAddISCSIPoolSourceHost: move to storage/storage_driver.c
   and rename to virStorageAddISCSIPoolSourceHost,
 - qemuTranslateDiskSourcePoolAuth: move to storage/storage_driver.c
   and rename to virStorageTranslateDiskSourcePoolAuth,
 - Update users of qemuTranslateDiskSourcePool to use a
   new name.

src/qemu/qemu_conf.c

@@ -1225,249 +1225,6 @@ int qemuDriverAllocateID(virQEMUDriverPtr driver)
     return virAtomicIntInc(&driver->nextvmid);
 }
 
-static int
-qemuAddISCSIPoolSourceHost(virDomainDiskDefPtr def,
-                           virStoragePoolDefPtr pooldef)
-{
-    int ret = -1;
-    char **tokens = NULL;
-
-    /* Only support one host */
-    if (pooldef->source.nhost != 1) {
-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                       _("Expected exactly 1 host for the storage pool"));
-        goto cleanup;
-    }
-
-    /* iscsi pool only supports one host */
-    def->src->nhosts = 1;
-
-    if (VIR_ALLOC_N(def->src->hosts, def->src->nhosts) < 0)
-        goto cleanup;
-
-    if (VIR_STRDUP(def->src->hosts[0].name, pooldef->source.hosts[0].name) < 0)
-        goto cleanup;
-
-    if (virAsprintf(&def->src->hosts[0].port, "%d",
-                    pooldef->source.hosts[0].port ?
-                    pooldef->source.hosts[0].port :
-                    3260) < 0)
-        goto cleanup;
-
-    /* iscsi volume has name like "unit:0:0:1" */
-    if (!(tokens = virStringSplit(def->src->srcpool->volume, ":", 0)))
-        goto cleanup;
-
-    if (virStringListLength(tokens) != 4) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("unexpected iscsi volume name '%s'"),
-                       def->src->srcpool->volume);
-        goto cleanup;
-    }
-
-    /* iscsi pool has only one source device path */
-    if (virAsprintf(&def->src->path, "%s/%s",
-                    pooldef->source.devices[0].path,
-                    tokens[3]) < 0)
-        goto cleanup;
-
-    /* Storage pool have not supported these 2 attributes yet,
-     * use the defaults.
-     */
-    def->src->hosts[0].transport = VIR_STORAGE_NET_HOST_TRANS_TCP;
-    def->src->hosts[0].socket = NULL;
-
-    def->src->protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI;
-
-    ret = 0;
-
- cleanup:
-    virStringFreeList(tokens);
-    return ret;
-}
-
-static int
-qemuTranslateDiskSourcePoolAuth(virDomainDiskDefPtr def,
-                                virStoragePoolSourcePtr source)
-{
-    int ret = -1;
-
-    /* Only necessary when authentication set */
-    if (!source->auth) {
-        ret = 0;
-        goto cleanup;
-    }
-    def->src->auth = virStorageAuthDefCopy(source->auth);
-    if (!def->src->auth)
-        goto cleanup;
-    ret = 0;
-
- cleanup:
-    return ret;
-}
-
-
-int
-qemuTranslateDiskSourcePool(virConnectPtr conn,
-                            virDomainDiskDefPtr def)
-{
-    virStoragePoolDefPtr pooldef = NULL;
-    virStoragePoolPtr pool = NULL;
-    virStorageVolPtr vol = NULL;
-    char *poolxml = NULL;
-    virStorageVolInfo info;
-    int ret = -1;
-    virErrorPtr savedError = NULL;
-
-    if (def->src->type != VIR_STORAGE_TYPE_VOLUME)
-        return 0;
-
-    if (!def->src->srcpool)
-        return 0;
-
-    if (!(pool = virStoragePoolLookupByName(conn, def->src->srcpool->pool)))
-        return -1;
-
-    if (virStoragePoolIsActive(pool) != 1) {
-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                       _("storage pool '%s' containing volume '%s' "
-                         "is not active"),
-                       def->src->srcpool->pool, def->src->srcpool->volume);
-        goto cleanup;
-    }
-
-    if (!(vol = virStorageVolLookupByName(pool, def->src->srcpool->volume)))
-        goto cleanup;
-
-    if (virStorageVolGetInfo(vol, &info) < 0)
-        goto cleanup;
-
-    if (!(poolxml = virStoragePoolGetXMLDesc(pool, 0)))
-        goto cleanup;
-
-    if (!(pooldef = virStoragePoolDefParseString(poolxml)))
-        goto cleanup;
-
-    def->src->srcpool->pooltype = pooldef->type;
-    def->src->srcpool->voltype = info.type;
-
-    if (def->src->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"));
-        goto cleanup;
-    }
-
-    VIR_FREE(def->src->path);
-    virStorageNetHostDefFree(def->src->nhosts, def->src->hosts);
-    virStorageAuthDefFree(def->src->auth);
-
-    switch ((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:
-    case VIR_STORAGE_POOL_ZFS:
-        if (!(def->src->path = virStorageVolGetPath(vol)))
-            goto cleanup;
-
-        if (def->startupPolicy && info.type != VIR_STORAGE_VOL_FILE) {
-            virReportError(VIR_ERR_XML_ERROR, "%s",
-                           _("'startupPolicy' is only valid for "
-                             "'file' type volume"));
-            goto cleanup;
-        }
-
-
-        switch (info.type) {
-        case VIR_STORAGE_VOL_FILE:
-            def->src->srcpool->actualtype = VIR_STORAGE_TYPE_FILE;
-            break;
-
-        case VIR_STORAGE_VOL_DIR:
-            def->src->srcpool->actualtype = VIR_STORAGE_TYPE_DIR;
-            break;
-
-        case VIR_STORAGE_VOL_BLOCK:
-            def->src->srcpool->actualtype = VIR_STORAGE_TYPE_BLOCK;
-            break;
-
-        case VIR_STORAGE_VOL_NETWORK:
-        case VIR_STORAGE_VOL_NETDIR:
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("unexpected storage volume type '%s' "
-                             "for storage pool type '%s'"),
-                           virStorageVolTypeToString(info.type),
-                           virStoragePoolTypeToString(pooldef->type));
-            goto cleanup;
-        }
-
-        break;
-
-    case VIR_STORAGE_POOL_ISCSI:
-        if (def->startupPolicy) {
-            virReportError(VIR_ERR_XML_ERROR, "%s",
-                           _("'startupPolicy' is only valid for "
-                             "'file' type volume"));
-            goto cleanup;
-        }
-
-       switch (def->src->srcpool->mode) {
-       case VIR_STORAGE_SOURCE_POOL_MODE_DEFAULT:
-       case VIR_STORAGE_SOURCE_POOL_MODE_LAST:
-           def->src->srcpool->mode = VIR_STORAGE_SOURCE_POOL_MODE_HOST;
-           /* fallthrough */
-       case VIR_STORAGE_SOURCE_POOL_MODE_HOST:
-           def->src->srcpool->actualtype = VIR_STORAGE_TYPE_BLOCK;
-           if (!(def->src->path = virStorageVolGetPath(vol)))
-               goto cleanup;
-           break;
-
-       case VIR_STORAGE_SOURCE_POOL_MODE_DIRECT:
-           def->src->srcpool->actualtype = VIR_STORAGE_TYPE_NETWORK;
-           def->src->protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI;
-
-           if (qemuTranslateDiskSourcePoolAuth(def, &pooldef->source) < 0)
-               goto cleanup;
-
-           if (qemuAddISCSIPoolSourceHost(def, pooldef) < 0)
-               goto cleanup;
-           break;
-       }
-       break;
-
-    case VIR_STORAGE_POOL_MPATH:
-    case VIR_STORAGE_POOL_RBD:
-    case VIR_STORAGE_POOL_SHEEPDOG:
-    case VIR_STORAGE_POOL_GLUSTER:
-    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;
-    }
-
-    ret = 0;
- cleanup:
-    if (ret < 0)
-        savedError = virSaveLastError();
-    if (pool)
-        virStoragePoolFree(pool);
-    if (vol)
-        virStorageVolFree(vol);
-    if (savedError) {
-        virSetError(savedError);
-        virFreeError(savedError);
-    }
-
-    VIR_FREE(poolxml);
-    virStoragePoolDefFree(pooldef);
-    return ret;
-}
-
 
 int
 qemuTranslateSnapshotDiskSourcePool(virConnectPtr conn ATTRIBUTE_UNUSED,

src/qemu/qemu_conf.h

@@ -307,9 +307,6 @@ int qemuSetUnprivSGIO(virDomainDeviceDefPtr dev);
 int qemuDriverAllocateID(virQEMUDriverPtr driver);
 virDomainXMLOptionPtr virQEMUDriverCreateXMLConf(virQEMUDriverPtr driver);
 
-int qemuTranslateDiskSourcePool(virConnectPtr conn,
-                                virDomainDiskDefPtr def);
-
 int qemuTranslateSnapshotDiskSourcePool(virConnectPtr conn,
                                         virDomainSnapshotDiskDefPtr def);
 

src/qemu/qemu_driver.c

@@ -6642,7 +6642,7 @@ qemuDomainChangeDiskMediaLive(virConnectPtr conn,
     virCapsPtr caps = NULL;
     int ret = -1;
 
-    if (qemuTranslateDiskSourcePool(conn, disk) < 0)
+    if (virStorageTranslateDiskSourcePool(conn, disk) < 0)
         goto end;
 
     if (qemuDomainDetermineDiskChain(driver, vm, disk, false) < 0)
@@ -12588,7 +12588,7 @@ qemuDomainSnapshotPrepareDiskExternal(virConnectPtr conn,
         return -1;
 
     if (!active) {
-        if (qemuTranslateDiskSourcePool(conn, disk) < 0)
+        if (virStorageTranslateDiskSourcePool(conn, disk) < 0)
             return -1;
 
         if (qemuDomainSnapshotPrepareDiskExternalBackingInactive(disk) < 0)
@@ -12646,7 +12646,7 @@ qemuDomainSnapshotPrepareDiskInternal(virConnectPtr conn,
     if (active)
         return 0;
 
-    if (qemuTranslateDiskSourcePool(conn, disk) < 0)
+    if (virStorageTranslateDiskSourcePool(conn, disk) < 0)
         return -1;
 
     actualType = virStorageSourceGetActualType(disk->src);

src/qemu/qemu_hotplug.c

@@ -49,6 +49,7 @@
 #include "virstoragefile.h"
 #include "virstring.h"
 #include "virtime.h"
+#include "storage/storage_driver.h"
 
 #define VIR_FROM_THIS VIR_FROM_QEMU
 
@@ -722,7 +723,7 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
         goto end;
     }
 
-    if (qemuTranslateDiskSourcePool(conn, disk) < 0)
+    if (virStorageTranslateDiskSourcePool(conn, disk) < 0)
         goto end;
 
     if (qemuAddSharedDevice(driver, dev, vm->def->name) < 0)

src/qemu/qemu_process.c

@@ -66,6 +66,7 @@
 #include "virnuma.h"
 #include "virstring.h"
 #include "virhostdev.h"
+#include "storage/storage_driver.h"
 
 #define VIR_FROM_THIS VIR_FROM_QEMU
 
@@ -3327,7 +3328,7 @@ qemuProcessReconnect(void *opaque)
     for (i = 0; i < obj->def->ndisks; i++) {
         virDomainDeviceDef dev;
 
-        if (qemuTranslateDiskSourcePool(conn, obj->def->disks[i]) < 0)
+        if (virStorageTranslateDiskSourcePool(conn, obj->def->disks[i]) < 0)
             goto error;
 
         /* XXX we should be able to restore all data from XML in the future */
@@ -4011,7 +4012,7 @@ int qemuProcessStart(virConnectPtr conn,
      * cgroup and security setting.
      */
     for (i = 0; i < vm->def->ndisks; i++) {
-        if (qemuTranslateDiskSourcePool(conn, vm->def->disks[i]) < 0)
+        if (virStorageTranslateDiskSourcePool(conn, vm->def->disks[i]) < 0)
             goto cleanup;
     }
 

src/storage/storage_driver.c

@@ -2908,3 +2908,248 @@ virStorageFileGetMetadata(virStorageSourcePtr src,
     virHashFree(cycle);
     return ret;
 }
+
+
+static int
+virStorageAddISCSIPoolSourceHost(virDomainDiskDefPtr def,
+                                 virStoragePoolDefPtr pooldef)
+{
+    int ret = -1;
+    char **tokens = NULL;
+
+    /* Only support one host */
+    if (pooldef->source.nhost != 1) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                       _("Expected exactly 1 host for the storage pool"));
+        goto cleanup;
+    }
+
+    /* iscsi pool only supports one host */
+    def->src->nhosts = 1;
+
+    if (VIR_ALLOC_N(def->src->hosts, def->src->nhosts) < 0)
+        goto cleanup;
+
+    if (VIR_STRDUP(def->src->hosts[0].name, pooldef->source.hosts[0].name) < 0)
+        goto cleanup;
+
+    if (virAsprintf(&def->src->hosts[0].port, "%d",
+                    pooldef->source.hosts[0].port ?
+                    pooldef->source.hosts[0].port :
+                    3260) < 0)
+        goto cleanup;
+
+    /* iscsi volume has name like "unit:0:0:1" */
+    if (!(tokens = virStringSplit(def->src->srcpool->volume, ":", 0)))
+        goto cleanup;
+
+    if (virStringListLength(tokens) != 4) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("unexpected iscsi volume name '%s'"),
+                       def->src->srcpool->volume);
+        goto cleanup;
+    }
+
+    /* iscsi pool has only one source device path */
+    if (virAsprintf(&def->src->path, "%s/%s",
+                    pooldef->source.devices[0].path,
+                    tokens[3]) < 0)
+        goto cleanup;
+
+    /* Storage pool have not supported these 2 attributes yet,
+     * use the defaults.
+     */
+    def->src->hosts[0].transport = VIR_STORAGE_NET_HOST_TRANS_TCP;
+    def->src->hosts[0].socket = NULL;
+
+    def->src->protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI;
+
+    ret = 0;
+
+ cleanup:
+    virStringFreeList(tokens);
+    return ret;
+}
+
+
+static int
+virStorageTranslateDiskSourcePoolAuth(virDomainDiskDefPtr def,
+                                      virStoragePoolSourcePtr source)
+{
+    int ret = -1;
+
+    /* Only necessary when authentication set */
+    if (!source->auth) {
+        ret = 0;
+        goto cleanup;
+    }
+    def->src->auth = virStorageAuthDefCopy(source->auth);
+    if (!def->src->auth)
+        goto cleanup;
+    ret = 0;
+
+ cleanup:
+    return ret;
+}
+
+
+int
+virStorageTranslateDiskSourcePool(virConnectPtr conn,
+                                  virDomainDiskDefPtr def)
+{
+    virStoragePoolDefPtr pooldef = NULL;
+    virStoragePoolPtr pool = NULL;
+    virStorageVolPtr vol = NULL;
+    char *poolxml = NULL;
+    virStorageVolInfo info;
+    int ret = -1;
+    virErrorPtr savedError = NULL;
+
+    if (def->src->type != VIR_STORAGE_TYPE_VOLUME)
+        return 0;
+
+    if (!def->src->srcpool)
+        return 0;
+
+    if (!(pool = virStoragePoolLookupByName(conn, def->src->srcpool->pool)))
+        return -1;
+
+    if (virStoragePoolIsActive(pool) != 1) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("storage pool '%s' containing volume '%s' "
+                         "is not active"),
+                       def->src->srcpool->pool, def->src->srcpool->volume);
+        goto cleanup;
+    }
+
+    if (!(vol = virStorageVolLookupByName(pool, def->src->srcpool->volume)))
+        goto cleanup;
+
+    if (virStorageVolGetInfo(vol, &info) < 0)
+        goto cleanup;
+
+    if (!(poolxml = virStoragePoolGetXMLDesc(pool, 0)))
+        goto cleanup;
+
+    if (!(pooldef = virStoragePoolDefParseString(poolxml)))
+        goto cleanup;
+
+    def->src->srcpool->pooltype = pooldef->type;
+    def->src->srcpool->voltype = info.type;
+
+    if (def->src->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"));
+        goto cleanup;
+    }
+
+    VIR_FREE(def->src->path);
+    virStorageNetHostDefFree(def->src->nhosts, def->src->hosts);
+    virStorageAuthDefFree(def->src->auth);
+
+    switch ((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:
+    case VIR_STORAGE_POOL_ZFS:
+        if (!(def->src->path = virStorageVolGetPath(vol)))
+            goto cleanup;
+
+        if (def->startupPolicy && info.type != VIR_STORAGE_VOL_FILE) {
+            virReportError(VIR_ERR_XML_ERROR, "%s",
+                           _("'startupPolicy' is only valid for "
+                             "'file' type volume"));
+            goto cleanup;
+        }
+
+
+        switch (info.type) {
+        case VIR_STORAGE_VOL_FILE:
+            def->src->srcpool->actualtype = VIR_STORAGE_TYPE_FILE;
+            break;
+
+        case VIR_STORAGE_VOL_DIR:
+            def->src->srcpool->actualtype = VIR_STORAGE_TYPE_DIR;
+            break;
+
+        case VIR_STORAGE_VOL_BLOCK:
+            def->src->srcpool->actualtype = VIR_STORAGE_TYPE_BLOCK;
+            break;
+
+        case VIR_STORAGE_VOL_NETWORK:
+        case VIR_STORAGE_VOL_NETDIR:
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("unexpected storage volume type '%s' "
+                             "for storage pool type '%s'"),
+                           virStorageVolTypeToString(info.type),
+                           virStoragePoolTypeToString(pooldef->type));
+            goto cleanup;
+        }
+
+        break;
+
+    case VIR_STORAGE_POOL_ISCSI:
+        if (def->startupPolicy) {
+            virReportError(VIR_ERR_XML_ERROR, "%s",
+                           _("'startupPolicy' is only valid for "
+                             "'file' type volume"));
+            goto cleanup;
+        }
+
+       switch (def->src->srcpool->mode) {
+       case VIR_STORAGE_SOURCE_POOL_MODE_DEFAULT:
+       case VIR_STORAGE_SOURCE_POOL_MODE_LAST:
+           def->src->srcpool->mode = VIR_STORAGE_SOURCE_POOL_MODE_HOST;
+           /* fallthrough */
+       case VIR_STORAGE_SOURCE_POOL_MODE_HOST:
+           def->src->srcpool->actualtype = VIR_STORAGE_TYPE_BLOCK;
+           if (!(def->src->path = virStorageVolGetPath(vol)))
+               goto cleanup;
+           break;
+
+       case VIR_STORAGE_SOURCE_POOL_MODE_DIRECT:
+           def->src->srcpool->actualtype = VIR_STORAGE_TYPE_NETWORK;
+           def->src->protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI;
+
+           if (virStorageTranslateDiskSourcePoolAuth(def, &pooldef->source) < 0)
+               goto cleanup;
+
+           if (virStorageAddISCSIPoolSourceHost(def, pooldef) < 0)
+               goto cleanup;
+           break;
+       }
+       break;
+
+    case VIR_STORAGE_POOL_MPATH:
+    case VIR_STORAGE_POOL_RBD:
+    case VIR_STORAGE_POOL_SHEEPDOG:
+    case VIR_STORAGE_POOL_GLUSTER:
+    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;
+    }
+
+    ret = 0;
+ cleanup:
+    if (ret < 0)
+        savedError = virSaveLastError();
+    if (pool)
+        virStoragePoolFree(pool);
+    if (vol)
+        virStorageVolFree(vol);
+    if (savedError) {
+        virSetError(savedError);
+        virFreeError(savedError);
+    }
+
+    VIR_FREE(poolxml);
+    virStoragePoolDefFree(pooldef);
+    return ret;
+}

src/storage/storage_driver.h

@@ -26,6 +26,7 @@
 
 # include <sys/stat.h>
 
+# include "domain_conf.h"
 # include "storage_conf.h"
 # include "virstoragefile.h"
 
@@ -52,6 +53,9 @@ int virStorageFileGetMetadata(virStorageSourcePtr src,
                               bool allow_probe)
     ATTRIBUTE_NONNULL(1);
 
+int virStorageTranslateDiskSourcePool(virConnectPtr conn,
+                                      virDomainDiskDefPtr def);
+
 int storageRegister(void);
 
 #endif /* __VIR_STORAGE_DRIVER_H__ */

tests/qemuxml2argvtest.c

@@ -21,6 +21,7 @@
 # include "conf/storage_conf.h"
 # include "cpu/cpu_map.h"
 # include "virstring.h"
+# include "storage/storage_driver.h"
 
 # include "testutilsqemu.h"
 
@@ -351,7 +352,7 @@ static int testCompareXMLToArgvFiles(const char *xml,
     }
 
     for (i = 0; i < vmdef->ndisks; i++) {
-        if (qemuTranslateDiskSourcePool(conn, vmdef->disks[i]) < 0)
+        if (virStorageTranslateDiskSourcePool(conn, vmdef->disks[i]) < 0)
             goto out;
     }