Re: [libvirt] [PATCH 3/6] qemu: Translate the iscsi pool/volume disk source

The difference with already supported pool types (dir, fs, block) is: there are two modes for iscsi pool (or network pools in future), one can specify it either to use the volume target path (the path showed up on host) with mode='host', or to use the remote URI qemu supports (e.g. file=iscsi://example.org:6000/iqn.1992-01.com.example/1) with mode='uri'. For 'host' mode, it copies the volume target path into disk->src. For 'uri' mode, the conresponded info in the *one* pool source host def

are copied to disk->hosts[0]. * src/conf/domain_conf.[ch]: (Introduce a new helper to check if the disk source is of block type: virDomainDiskSourceIsBlockType; Introduce "pooltype" for disk->srcpool, to indicate the pool type) src/libvirt_private.syms: (Expose the new helper's symbol) * src/qemu/qemu_conf.c: (Use the helper to ease the checking in "shared disk", "sgio" related helpers; Translate the iscsi pool/volume disk source) * src/qemu/qemu_command.c (Use the helper to ease the checking in qemuBuildDriveDevStr; Build qemu command line for iscsi pool/volume disk source) --- src/conf/domain_conf.c | 42 +++++++++++++++++ src/conf/domain_conf.h | 3 ++ src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 20 ++++++-- src/qemu/qemu_conf.c | 117 ++++++++++++++++++++++++++++++++++++++--------- 5 files changed, 158 insertions(+), 25 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 009a8aa..04b14dc 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -41,6 +41,7 @@ #include "virbuffer.h" #include "virlog.h" #include "nwfilter_conf.h" +#include "storage_conf.h" #include "virstoragefile.h" #include "virfile.h" #include "virbitmap.h" @@ -17950,3 +17951,44 @@ virDomainDiskDefGenSecurityLabelDef(const char *model) return seclabel; } + +/** + * virDomainDiskSourceIsBlockType: + * + * Check if the disk *source* is of block type. This just tries + * to check from the type of disk def, not to probe the underlying + * storage. + * + * Return true if its source is block type, or false otherwise. + */ +bool +virDomainDiskSourceIsBlockType(virDomainDiskDefPtr def) +{ + if (!def) + return false; + + /* No reason to think the disk source is block type if + * the source is empty + */ + if (!def->src && !def->srcpool) + return false;

+ + if (def->type == VIR_DOMAIN_DISK_TYPE_BLOCK) + return true; + + if (def->type == VIR_DOMAIN_DISK_TYPE_VOLUME) { + if (def->srcpool->voltype == VIR_STORAGE_VOL_BLOCK) {

+ /* We don't think the volume accessed by remote URI is + * block type source, since we can't/shoudn't manage it

+ * (e.g. set sgio=filtered|unfilered for it) in libvirt.

+ */ + if (def->srcpool->pooltype == VIR_STORAGE_POOL_ISCSI && + def->srcpool->mode == VIR_DOMAIN_DISK_SOURCE_POOL_MODE_URI) + return false; + + return true; + } + } + + return false; +} diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index d57b7c3..3c60293 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -671,6 +671,7 @@ struct _virDomainDiskSourcePoolDef { char *pool; /* pool name */ char *volume; /* volume name */ int voltype; /* enum virStorageVolType, internal only */ + int pooltype; /* enum virStoragePoolType, internal only */

int mode; /* enum virDomainDiskSourcePoolMode */ }; typedef virDomainDiskSourcePoolDef *virDomainDiskSourcePoolDefPtr; @@ -2652,4 +2653,6 @@ virDomainDefMaybeAddController(virDomainDefPtr def, char *virDomainDefGetDefaultEmulator(virDomainDefPtr def, virCapsPtr caps); +bool virDomainDiskSourceIsBlockType(virDomainDiskDefPtr def); + #endif /* __DOMAIN_CONF_H */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1ea7467..fa9f079 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -161,6 +161,7 @@ virDomainDiskProtocolTransportTypeToString; virDomainDiskProtocolTypeToString; virDomainDiskRemove; virDomainDiskRemoveByName; +virDomainDiskSourceIsBlockType; virDomainDiskTypeFromString; virDomainDiskTypeToString; virDomainEmulatorPinAdd; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 486682e..ae5f7dd 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -42,6 +42,7 @@ #include "domain_audit.h" #include "domain_conf.h" #include "snapshot_conf.h" +#include "storage_conf.h" #include "network/bridge_driver.h" #include "virnetdevtap.h" #include "base64.h" @@ -3163,7 +3164,20 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, "block type volume")); goto error; } - virBufferEscape(&opt, ',', ",", "file=%s,", disk->src); + + if (disk->srcpool->pooltype == VIR_STORAGE_POOL_ISCSI) { + if (disk->srcpool->mode == + VIR_DOMAIN_DISK_SOURCE_POOL_MODE_URI) { + if (qemuBuildISCSIString(conn, disk, &opt) < 0) + goto error; + 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: virBufferEscape(&opt, ',', ",", "file=%s,", disk->src); @@ -3450,9 +3464,7 @@ qemuBuildDriveDevStr(virDomainDefPtr def, virDomainDiskProtocolTypeToString(disk->protocol)); goto error; } - } else if (disk->type != VIR_DOMAIN_DISK_TYPE_BLOCK && - !(disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME && - disk->srcpool->voltype == VIR_STORAGE_VOL_BLOCK)) { + } else if (!virDomainDiskSourceIsBlockType(disk)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("disk device='lun' is only valid for block type disk source")); goto error; diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 094f9f7..b67f182 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -52,6 +52,7 @@ #include "virfile.h" #include "virstring.h" #include "viratomic.h" +#include "storage_conf.h" #include "configmake.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -1180,12 +1181,7 @@ qemuAddSharedDevice(virQEMUDriverPtr driver, if (dev->type == VIR_DOMAIN_DEVICE_DISK) { disk = dev->data.disk; - if (!disk->shared || - !disk->src || - (disk->type != VIR_DOMAIN_DISK_TYPE_BLOCK && - !(disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME && - disk->srcpool && - disk->srcpool->voltype == VIR_STORAGE_VOL_BLOCK))) + if (!disk->shared || !virDomainDiskSourceIsBlockType(disk)) return 0; } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) { hostdev = dev->data.hostdev; @@ -1295,12 +1291,7 @@ qemuRemoveSharedDevice(virQEMUDriverPtr driver, if (dev->type == VIR_DOMAIN_DEVICE_DISK) { disk = dev->data.disk; - if (!disk->shared || - !disk->src || - (disk->type != VIR_DOMAIN_DISK_TYPE_BLOCK && - !(disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME && - disk->srcpool && - disk->srcpool->voltype == VIR_STORAGE_VOL_BLOCK))) + if (!disk->shared || !virDomainDiskSourceIsBlockType(disk)) return 0; } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) { hostdev = dev->data.hostdev; @@ -1392,12 +1383,8 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev) if (dev->type == VIR_DOMAIN_DEVICE_DISK) { disk = dev->data.disk; - if (!disk->src || - disk->device != VIR_DOMAIN_DISK_DEVICE_LUN || - (disk->type != VIR_DOMAIN_DISK_TYPE_BLOCK && - !(disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME && - disk->srcpool && - disk->srcpool->voltype == VIR_STORAGE_VOL_BLOCK))) + if (disk->device != VIR_DOMAIN_DISK_DEVICE_LUN || + virDomainDiskSourceIsBlockType(disk)) return 0; path = disk->src; @@ -1463,9 +1450,12 @@ int qemuTranslateDiskSourcePool(virConnectPtr conn, virDomainDiskDefPtr def) { + virStoragePoolDefPtr pooldef = NULL; virStoragePoolPtr pool = NULL; virStorageVolPtr vol = NULL; + char *poolxml = NULL; virStorageVolInfo info; + char **tokens = NULL; int ret = -1; if (def->type != VIR_DOMAIN_DISK_TYPE_VOLUME)

@@ -1492,11 +1482,91 @@ qemuTranslateDiskSourcePool(virConnectPtr conn, switch (info.type) { case VIR_STORAGE_VOL_FILE: - case VIR_STORAGE_VOL_BLOCK: case VIR_STORAGE_VOL_DIR: 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 (pooldef->type != VIR_STORAGE_POOL_ISCSI && + def->srcpool->mode) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("disk source mode is only valid when " + "storage pool is of iscsi type")); + goto cleanup; + } + + 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_URI) { + /* iscsi pool only supports one host */ + def->nhosts = 1; + + if (VIR_ALLOC_N(def->hosts, def->nhosts) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (VIR_STRDUP(def->hosts[0].name, + pooldef->source.hosts[0].name) < 0) + goto cleanup; + + if (virAsprintf(&def->hosts[0].port, "%d", + pooldef->source.hosts[0].port ? + pooldef->source.hosts[0].port : + 3260) < 0) {

From 1/6 in storage_backend_iscsi:

+ virReportOOMError(); + goto cleanup; + } + + /* iscsi volume has name like "unit:0:0:1" */ + if (!(tokens = virStringSplit(def->srcpool->volume, + ":", 0)))

+ goto cleanup; + + if (virStringListLength(tokens) != 4) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected iscsi volume name '%s'"), + def->srcpool->volume); + goto cleanup; + } + + /* iscsi pool has only one source device path */

+ if (virAsprintf(&def->src, "%s/%s", + pooldef->source.devices[0].path, + tokens[3]) < 0) { + virReportOOMError(); + goto cleanup; + } + + /* Storage pool have not supportted these 2 attributes yet, + * use the defaults.

+ */ + def->hosts[0].transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP; + def->hosts[0].socket = NULL; + + def->protocol = VIR_DOMAIN_DISK_PROTOCOL_ISCSI; + } else if (def->srcpool->mode == + VIR_DOMAIN_DISK_SOURCE_POOL_MODE_HOST) { + if (!(def->src = virStorageVolGetPath(vol))) + goto cleanup; + } + } else { + if (!(def->src = virStorageVolGetPath(vol))) + goto cleanup; + } + + def->srcpool->pooltype = pooldef->type; + break; case VIR_STORAGE_VOL_NETWORK: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Using network volume as disk source is not supported"));

@@ -1506,7 +1576,12 @@ qemuTranslateDiskSourcePool(virConnectPtr conn, def->srcpool->voltype = info.type; ret = 0; cleanup: - virStoragePoolFree(pool); - virStorageVolFree(vol); + if (pool) + virStoragePoolFree(pool); + if (vol) + virStorageVolFree(vol);

+ VIR_FREE(poolxml); + virStoragePoolDefFree(pooldef); + virStringFreeList(tokens); return ret; }