On 04/04/2013 03:38 PM, Osier Yang wrote:
> To support "shareable" for volume type disk, we have to translate
> the source before trying to add the shared disk entry. To archieve
s/archeive/achieve/
> the goal, this moves the helper qemuTranslateDiskSourcePool into
> src/qemu/qemu_conf.c, and introduce a internal only member (voltype)
s/a internal/an internal/
> for struct _virDomainDiskSourcePoolDef, to record the underlying
> volume type, for use when building the drive string.
s/type,/type/
>
> Later patch will support "shareable" volume type disk.
> ---
> src/conf/domain_conf.h | 1 +
> src/qemu/qemu_command.c | 56 +------------------------------------------------
> src/qemu/qemu_command.h | 1 -
> src/qemu/qemu_conf.c | 52 +++++++++++++++++++++++++++++++++++++++++++++
> src/qemu/qemu_conf.h | 3 +++
> src/qemu/qemu_driver.c | 16 ++++++++++----
> src/qemu/qemu_process.c | 7 +++++++
> 7 files changed, 76 insertions(+), 60 deletions(-)
>
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 988636e..13d6d89 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -611,6 +611,7 @@ typedef struct _virDomainDiskSourcePoolDef virDomainDiskSourcePoolDef;
> struct _virDomainDiskSourcePoolDef {
> char *pool; /* pool name */
> char *volume; /* volume name */
> + int voltype; /* enum virStorageVolType, internal only */
> };
> typedef virDomainDiskSourcePoolDef *virDomainDiskSourcePoolDefPtr;
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index cdfe801..c29796d 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -2681,56 +2681,6 @@ 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;
> -
> - if (def->startupPolicy &&
> - info.type != VIR_STORAGE_VOL_FILE) {
> - virReportError(VIR_ERR_XML_ERROR, "%s",
> - _("'startupPolicy' is only valid for 'file' type volume"));
> - 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,
> @@ -2743,7 +2693,6 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
> virDomainDiskGeometryTransTypeToString(disk->geometry.trans);
> int idx = virDiskNameToIndex(disk->dst);
> int busid = -1, unitid = -1;
> - int voltype = -1;
>
> if (idx < 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -2751,9 +2700,6 @@ 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) {
> @@ -2889,7 +2835,7 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
> break;
> }
> } else if (disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME) {
> - switch (voltype) {
> + switch (disk->srcpool->voltype) {
> case VIR_STORAGE_VOL_DIR:
> if (!disk->readonly) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
> index 17687f4..20e3066 100644
> --- a/src/qemu/qemu_command.h
> +++ b/src/qemu/qemu_command.h
> @@ -237,5 +237,4 @@ qemuParseKeywords(const char *str,
> char ***retvalues,
> int allowEmptyValue);
>
> -
> #endif /* __QEMU_COMMAND_H__*/
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index c2e2e10..8ae690f 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -1240,3 +1240,55 @@ int qemuDriverAllocateID(virQEMUDriverPtr driver)
> {
> return virAtomicIntInc(&driver->nextvmid);
> }
> +
> +int
> +qemuTranslateDiskSourcePool(virConnectPtr conn,
> + virDomainDiskDefPtr def)
> +{
> + virStoragePoolPtr pool = NULL;
> + virStorageVolPtr vol = NULL;
> + virStorageVolInfo info;
> + int ret = -1;
> +
> + if (def->type != VIR_DOMAIN_DISK_TYPE_VOLUME)
> + return 0;
> +
> + if (!def->srcpool)
> + 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;
> +
> + if (def->startupPolicy &&
> + info.type != VIR_STORAGE_VOL_FILE) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("'startupPolicy' is only valid for 'file' type volume"));
> + 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;
> + }
> +
> + def->srcpool->voltype = info.type;
> + ret = 0;
> +cleanup:
> + virStoragePoolFree(pool);
> + virStorageVolFree(vol);
> + return ret;
> +}
> diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
> index c5ddaad..9f58454 100644
> --- a/src/qemu/qemu_conf.h
> +++ b/src/qemu/qemu_conf.h
> @@ -303,4 +303,7 @@ void qemuSharedDiskEntryFree(void *payload, const void *name)
> int qemuDriverAllocateID(virQEMUDriverPtr driver);
> virDomainXMLConfPtr virQEMUDriverCreateXMLConf(void);
>
> +int qemuTranslateDiskSourcePool(virConnectPtr conn,
> + virDomainDiskDefPtr def);
> +
> #endif /* __QEMUD_CONF_H */
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 552a81b..4e8c666 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -5748,6 +5748,9 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
> goto end;
> }
>
> + if (qemuTranslateDiskSourcePool(conn, disk) < 0)
> + goto end;
> +
> if (qemuAddSharedDisk(driver, disk, vm->def->name) < 0)
> goto end;
>
> @@ -6016,7 +6019,8 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
> }
>
> static int
> -qemuDomainChangeDiskMediaLive(virDomainObjPtr vm,
> +qemuDomainChangeDiskMediaLive(virConnectPtr conn,
> + virDomainObjPtr vm,
> virDomainDeviceDefPtr dev,
> virQEMUDriverPtr driver,
> bool force)
> @@ -6029,6 +6033,9 @@ qemuDomainChangeDiskMediaLive(virDomainObjPtr vm,
> virCapsPtr caps = NULL;
> int ret = -1;
>
> + if (qemuTranslateDiskSourcePool(conn, disk) < 0)
> + goto end;
> +
> if (qemuDomainDetermineDiskChain(driver, disk, false) < 0)
> goto end;
>
> @@ -6107,7 +6114,8 @@ end:
> }
>
> static int
> -qemuDomainUpdateDeviceLive(virDomainObjPtr vm,
> +qemuDomainUpdateDeviceLive(virConnectPtr conn,
> + virDomainObjPtr vm,
> virDomainDeviceDefPtr dev,
> virDomainPtr dom,
> bool force)
> @@ -6117,7 +6125,7 @@ qemuDomainUpdateDeviceLive(virDomainObjPtr vm,
>
> switch (dev->type) {
> case VIR_DOMAIN_DEVICE_DISK:
> - ret = qemuDomainChangeDiskMediaLive(vm, dev, driver, force);
> + ret = qemuDomainChangeDiskMediaLive(conn, vm, dev, driver, force);
> break;
> case VIR_DOMAIN_DEVICE_GRAPHICS:
> ret = qemuDomainChangeGraphics(driver, vm, dev->data.graphics);
> @@ -6529,7 +6537,7 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml,
> ret = qemuDomainDetachDeviceLive(vm, dev_copy, dom);
> break;
> case QEMU_DEVICE_UPDATE:
> - ret = qemuDomainUpdateDeviceLive(vm, dev_copy, dom, force);
> + ret = qemuDomainUpdateDeviceLive(dom->conn, vm, dev_copy, dom, force);
> break;
> default:
> virReportError(VIR_ERR_INTERNAL_ERROR,
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 8c4bfb7..3ac0c70 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -3008,6 +3008,8 @@ qemuProcessReconnect(void *opaque)
> * qemu_driver->sharedDisks.
> */
> for (i = 0; i < obj->def->ndisks; i++) {
> + if (qemuTranslateDiskSourcePool(conn, obj->def->disks[i]) < 0)
> + goto error;
> if (qemuAddSharedDisk(driver, obj->def->disks[i],
> obj->def->name) < 0)
> goto error;
> @@ -3556,6 +3558,11 @@ int qemuProcessStart(virConnectPtr conn,
> goto cleanup;
> }
>
> + for (i = 0; i < vm->def->ndisks; i++) {
> + if (qemuTranslateDiskSourcePool(conn, vm->def->disks[i]) < 0)
> + goto cleanup;
> + }
> +
> VIR_DEBUG("Building emulator command line");
> if (!(cmd = qemuBuildCommandLine(conn, driver, vm->def, priv->monConfig,
> priv->monJSON != 0, priv->qemuCaps,
>
ACK; however, I see paths to qemuBuildDriveStr() that don't go through
qemuBuildCommandLine(), so just double check that you aren't missing a
place to get that disk->src set for one of these pool/vol's. The point
being that qemuBuildDriveStr() "had" code that did something before
(albeit in this set of patches). Since you're moving it - just be sure
there's no path that could enter qemuBuildDriveStr() that would need it.
John
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list