On 26.11.2013 17:48, Peter Krempa wrote:
> 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'>
> <seclabel model='selinux' relabel='yes'>
> <label>system_u:system_r:public_content_t:s0</label>
> </seclabel>
> @@ -25,7 +25,7 @@
> <address type='drive' controller='0' bus='0' target='0' unit='1'/>
> </disk>
> <disk type='volume' device='cdrom'>
> - <source pool='blk-pool0' volume='blk-pool0-vol1' mode='direct' startupPolicy='optional'>
Just curious, I see you're removing startupPolicy here and in other
patches. Would it hurt to leave it there?
At any rate, ACK.
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list