Re: [libvirt] [PATCH 3/8] qemu: Translate the pool disk source when building drive string

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(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list