[libvirt] [PATCH 6/8] qemu: Translate the pool disk source earlier
Osier Yang
jyang at redhat.com
Mon Apr 8 11:08:18 UTC 2013
On 06/04/13 08:34, John Ferlan wrote:
> 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.
>
Yeah, I expect there are still places missed, but as I replied for 0/8.
Let's find them out step by step.
Osier
More information about the libvir-list
mailing list