On 04/05/2013 05:24 PM, John Ferlan wrote:
> 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).
One huge benefit of pool designations is that you can specify a domain
that uses volume "foo" within pool "bar", and migrate it between two
machines where pool "bar" has two different locations for the two
machines (maybe /mnt/nfs/bar on one machine and /longer/path/to/bar on
the other), and the XML on the destination will automatically find the
absolute path to foo relative to the pool bar. If you specify with disk
type='file', you have to specify an absolute path in your XML, and on
migration, if the destination doesn't have the same absolute path, you
are forced to pass alternate xml that you manage yourself to touch up
the path as part of the migration.
>
> 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
>
>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list