On 04/04/2013 03:37 PM, Osier Yang wrote:
> This adds a new helper qemuTranslateDiskSourcePool which uses the
> storage pool/vol APIs to tranlsate the disk source before building
s/tranlsate/translate/
> the drive string. Network volume is not supported yet. Disk chain
> for volume type disk may be supported later, but before I'm confident
> it doesn't break anything, it's just disabled now.
How would 'network volume' differ from "<disk type='network'"? And all
the variants therein?
Still trying to figure the 'benefit' of the volume tag since each of the
types looked up in the pool could also be specified as "<disk
type='xxx'" types (where xxx is file, block, dir).
But I'll press on with at least a mechanical review. Maybe someone else
can chime in with the product level viewpoint.
> ---
> src/qemu/qemu_command.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++
> src/qemu/qemu_domain.c | 4 ++-
> 2 files changed, 82 insertions(+), 1 deletion(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 693d30d..03c7195 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -2681,6 +2681,49 @@ qemuBuildNBDString(virConnectPtr conn, virDomainDiskDefPtr disk, virBufferPtr op
> return 0;
> }
>
> +static int
> +qemuTranslateDiskSourcePool(virConnectPtr conn,
> + virDomainDiskDefPtr def,
> + int *voltype)
> +{
> + virStoragePoolPtr pool = NULL;
> + virStorageVolPtr vol = NULL;
> + virStorageVolInfo info;
> + int ret = -1;
> +
> + if (def->type != VIR_DOMAIN_DISK_TYPE_VOLUME)
> + return 0;
> +
> + if (!(pool = virStoragePoolLookupByName(conn, def->srcpool->pool)))
> + return -1;
> +
> + if (!(vol = virStorageVolLookupByName(pool, def->srcpool->volume)))
> + goto cleanup;
> +
> + if (virStorageVolGetInfo(vol, &info) < 0)
> + goto cleanup;
> +
> + 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_NETWORK:
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("Using network volume as disk source is not supported"));
> + goto cleanup;
> + }
> +
> + *voltype = info.type;
> + ret = 0;
> +cleanup:
> + virStoragePoolFree(pool);
> + virStorageVolFree(vol);
> + return ret;
> +}
> +
> char *
> qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
> virDomainDiskDefPtr disk,
> @@ -2693,6 +2736,7 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
> virDomainDiskGeometryTransTypeToString(disk->geometry.trans);
> int idx = virDiskNameToIndex(disk->dst);
> int busid = -1, unitid = -1;
> + int voltype = -1;
Does initializing this matter? Ah perhaps the compiler complains...
>
> if (idx < 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -2700,6 +2744,9 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
> goto error;
> }
>
> + if (qemuTranslateDiskSourcePool(conn, disk, &voltype) < 0)
> + goto error;
> +
> switch (disk->bus) {
> case VIR_DOMAIN_DISK_BUS_SCSI:
> if (disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) {
> @@ -2834,6 +2881,38 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
> }
> break;
> }
> + } else if (disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME) {
> + switch (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 error;
> + }
> + 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 error;
> + }
> + virBufferEscape(&opt, ',', ",", "file=%s,", disk->src);
> + break;
> + case VIR_STORAGE_VOL_FILE:
> + virBufferEscape(&opt, ',', ",", "file=%s,", disk->src);
> + break;
> + case VIR_STORAGE_VOL_NETWORK:
> + /* Let the compiler shutup, qemuTranslateDiskSourcePool already
consider "keep the compiler quiet" :-) although I understand the
sentiment :-)
> + * reported the unsupported error.
> + */
> + break;
> + }
> } else {
> if ((disk->type == VIR_DOMAIN_DISK_TYPE_BLOCK) &&
> (disk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN)) {
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index c79b05d..6762152 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -1943,7 +1943,9 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
> virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> int ret = 0;
>
> - if (!disk->src || disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK)
> + if (!disk->src ||
> + disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK ||
> + disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME)
> goto cleanup;
>
> if (disk->backingChain) {
>
ACK, mechnically it seems what's here covers things.
John
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list