Re: [libvirt] [PATCHv1.5 04/27] qemuxml2argv: Add test for disk type='volume' with iSCSI pools

Tweak the existing file so that it can be tested for command line corectness.

--- src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 76 +----------- src/qemu/qemu_conf.c | 129 ++++++++++++++------- src/qemu/qemu_conf.h | 2 + .../qemuxml2argv-disk-source-pool-mode.args | 10 ++ .../qemuxml2argv-disk-source-pool-mode.xml | 4 +- tests/qemuxml2argvtest.c | 2 + 8 files changed, 112 insertions(+), 113 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.args 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 763417f..b8129a3 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3775,69 +3775,6 @@ qemuBuildNBDString(virConnectPtr conn, virDomainDiskDefPtr disk, virBufferPtr op return 0; } -static int -qemuBuildVolumeString(virConnectPtr conn, - virDomainDiskDefPtr disk, - virBufferPtr opt)

-{ - int ret = -1; - - switch ((virStorageVolType) 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: - case VIR_STORAGE_VOL_NETDIR: - case VIR_STORAGE_VOL_LAST: - /* Keep the compiler quiet, qemuTranslateDiskSourcePool already - * reported the unsupported error. - */ - goto cleanup; - } - - ret = 0; - -cleanup: - return ret; -} char * qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, @@ -3851,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, @@ -3934,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, @@ -3957,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) @@ -4025,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 77df370..58a0500 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,72 +1330,108 @@ 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 ((virStorageVolType) 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: + 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; } - 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: - case VIR_STORAGE_VOL_NETDIR: - case VIR_STORAGE_VOL_LAST: - 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_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; } - 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); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.args new file mode 100644 index 0000000..8f6a3dd --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.args @@ -0,0 +1,10 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults \ +-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -drive \ +file=/some/block/device/unit:0:0:1,if=none,media=cdrom,id=drive-ide0-0-1 -device \ +ide-drive,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 -drive \ +file=iscsi://iscsi.example.com:3260/demo-target/2,if=none,media=cdrom,id=drive-ide0-0-2 \ +-device ide-drive,bus=ide.0,unit=2,drive=drive-ide0-0-2,id=ide0-0-2 -drive \ +file=/tmp/idedisk.img,if=none,id=drive-ide0-0-3 -device \ +ide-drive,bus=ide.0,unit=3,drive=drive-ide0-0-3,id=ide0-0-3 -device \ +virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml index b907633..9f90293 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml @@ -15,7 +15,7 @@ <devices> <emulator>/usr/bin/qemu</emulator> <disk type='volume' device='cdrom'> - <source pool='blk-pool0' volume='blk-pool0-vol0' mode='host' startupPolicy='optional'> + <source pool='pool-iscsi-auth' volume='unit:0:0:1' mode='host'>