[libvirt] [PATCH 00/10] qemu: Refactor disk hotplug code for better reuse with blockdev (blockdev-add saga)
Move around few of the helpers which deal with disk addition and removal and fix a potential bug in the blockdev media change routine. The refactors will allow simplify a corner case of using blockdev such as if the disk frontend is unplugged the backends can be unplugged only after blockjobs terminate. This will allow simpler handling of the backing chain data in conjunction with blockjobs. To test the blockdev part you can use the new XML namespace element for qemu: <domain type='kvm' xmlns:qemu='http://libvirt.org/schemas/domain/qemu/1.0'> [...] <qemu:capabilities> <qemu:add capability='blockdev'/> </qemu:capabilities> if you have a recent enough qemu. Peter Krempa (10): qemu: block: Move and rename qemuHotplugRemoveStorageSourcePrepareData qemu: hotplug: Handle copy-on-read filter separate from rest of backing chain qemu: Introduce new set of helpers for attaching and detaching storage chains qemu: command: Use storage chain helpers in commandline generator qemu: command: Use VIR_AUTO infrastructure in qemuBuildDiskSourceCommandLine qemu: command: get rid of 'cleanup' in qemuBuildDiskSourceCommandLine qemu: hotplug: Use storage chain helpers in qemuDomainAttachDiskGeneric qemu: hotplug: Use storage chain helpers in qemuDomainRemoveDiskDevice qemu: hotplug: qemu: hotplug: Use storage chain helpers in qemuDomainChangeMediaBlockdev qemu: hotplug: Remove rest of source backend if hotplug fails src/qemu/qemu_block.c | 174 +++++++++++++++++++++ src/qemu/qemu_block.h | 33 ++++ src/qemu/qemu_command.c | 132 ++++++++++------ src/qemu/qemu_command.h | 11 ++ src/qemu/qemu_hotplug.c | 332 +++++++--------------------------------- 5 files changed, 361 insertions(+), 321 deletions(-) -- 2.21.0
Move it to qemu_block.c and call it qemuBlockStorageSourceDetachPrepare. It will be reused in other parts as well. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 57 +++++++++++++++++++++++++++++++++++++ src/qemu/qemu_block.h | 4 +++ src/qemu/qemu_hotplug.c | 63 ++--------------------------------------- 3 files changed, 63 insertions(+), 61 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 7d9f7ec3ab..560488c852 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1565,6 +1565,63 @@ qemuBlockStorageSourceAttachRollback(qemuMonitorPtr mon, } +/** + * qemuBlockStorageSourceDetachPrepare: + * @src: disk source structure + * @driveAlias: Alias of the -drive backend, the pointer is always consumed + * + * Prepare qemuBlockStorageSourceAttachDataPtr for detaching a single source + * from a VM. If @driveAlias is NULL -blockdev is assumed. + */ +qemuBlockStorageSourceAttachDataPtr +qemuBlockStorageSourceDetachPrepare(virStorageSourcePtr src, + char *driveAlias) +{ + qemuDomainStorageSourcePrivatePtr srcpriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src); + VIR_AUTOPTR(qemuBlockStorageSourceAttachData) data = NULL; + qemuBlockStorageSourceAttachDataPtr ret = NULL; + + if (VIR_ALLOC(data) < 0) + goto cleanup; + + if (driveAlias) { + VIR_STEAL_PTR(data->driveAlias, driveAlias); + data->driveAdded = true; + } else { + data->formatNodeName = src->nodeformat; + data->formatAttached = true; + data->storageNodeName = src->nodestorage; + data->storageAttached = true; + } + + if (src->pr && + !virStoragePRDefIsManaged(src->pr) && + VIR_STRDUP(data->prmgrAlias, src->pr->mgralias) < 0) + goto cleanup; + + if (VIR_STRDUP(data->tlsAlias, src->tlsAlias) < 0) + goto cleanup; + + if (srcpriv) { + if (srcpriv->secinfo && + srcpriv->secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES && + VIR_STRDUP(data->authsecretAlias, srcpriv->secinfo->s.aes.alias) < 0) + goto cleanup; + + if (srcpriv->encinfo && + srcpriv->encinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES && + VIR_STRDUP(data->encryptsecretAlias, srcpriv->encinfo->s.aes.alias) < 0) + goto cleanup; + } + + VIR_STEAL_PTR(ret, data); + + cleanup: + VIR_FREE(driveAlias); + return ret; +} + + /** * qemuBlockStorageSourceDetachOneBlockdev: * @driver: qemu driver object diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h index e41b9a1df2..a49c73670b 100644 --- a/src/qemu/qemu_block.h +++ b/src/qemu/qemu_block.h @@ -107,6 +107,10 @@ VIR_DEFINE_AUTOPTR_FUNC(qemuBlockStorageSourceAttachData, qemuBlockStorageSourceAttachDataPtr qemuBlockStorageSourceAttachPrepareBlockdev(virStorageSourcePtr src); +qemuBlockStorageSourceAttachDataPtr +qemuBlockStorageSourceDetachPrepare(virStorageSourcePtr src, + char *driveAlias); + int qemuBlockStorageSourceAttachApply(qemuMonitorPtr mon, qemuBlockStorageSourceAttachDataPtr data); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index efda539208..fe2f577d58 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -448,64 +448,6 @@ qemuHotplugDiskSourceDataFree(qemuHotplugDiskSourceDataPtr data) } -/** - * qemuDomainRemoveDiskStorageSourcePrepareData: - * @src: disk source structure - * @driveAlias: Alias of the -drive backend, the pointer is always consumed - * - * Prepare qemuBlockStorageSourceAttachDataPtr for detaching a single source - * from a VM. If @driveAlias is NULL -blockdev is assumed. - */ -static qemuBlockStorageSourceAttachDataPtr -qemuHotplugRemoveStorageSourcePrepareData(virStorageSourcePtr src, - char *driveAlias) - -{ - qemuDomainStorageSourcePrivatePtr srcpriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src); - VIR_AUTOPTR(qemuBlockStorageSourceAttachData) data = NULL; - qemuBlockStorageSourceAttachDataPtr ret = NULL; - - if (VIR_ALLOC(data) < 0) - goto cleanup; - - if (driveAlias) { - VIR_STEAL_PTR(data->driveAlias, driveAlias); - data->driveAdded = true; - } else { - data->formatNodeName = src->nodeformat; - data->formatAttached = true; - data->storageNodeName = src->nodestorage; - data->storageAttached = true; - } - - if (src->pr && - !virStoragePRDefIsManaged(src->pr) && - VIR_STRDUP(data->prmgrAlias, src->pr->mgralias) < 0) - goto cleanup; - - if (VIR_STRDUP(data->tlsAlias, src->tlsAlias) < 0) - goto cleanup; - - if (srcpriv) { - if (srcpriv->secinfo && - srcpriv->secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES && - VIR_STRDUP(data->authsecretAlias, srcpriv->secinfo->s.aes.alias) < 0) - goto cleanup; - - if (srcpriv->encinfo && - srcpriv->encinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES && - VIR_STRDUP(data->encryptsecretAlias, srcpriv->encinfo->s.aes.alias) < 0) - goto cleanup; - } - - VIR_STEAL_PTR(ret, data); - - cleanup: - VIR_FREE(driveAlias); - return ret; -} - - static qemuHotplugDiskSourceDataPtr qemuHotplugDiskSourceRemovePrepare(virDomainDiskDefPtr disk, virStorageSourcePtr src, @@ -526,7 +468,7 @@ qemuHotplugDiskSourceRemovePrepare(virDomainDiskDefPtr disk, goto cleanup; for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) { - if (!(backend = qemuHotplugRemoveStorageSourcePrepareData(n, NULL))) + if (!(backend = qemuBlockStorageSourceDetachPrepare(n, NULL))) goto cleanup; if (VIR_APPEND_ELEMENT(data->backends, data->nbackends, backend) < 0) @@ -536,8 +478,7 @@ qemuHotplugDiskSourceRemovePrepare(virDomainDiskDefPtr disk, if (!(drivealias = qemuAliasDiskDriveFromDisk(disk))) goto cleanup; - if (!(backend = qemuHotplugRemoveStorageSourcePrepareData(src, - drivealias))) + if (!(backend = qemuBlockStorageSourceDetachPrepare(src, drivealias))) goto cleanup; if (VIR_APPEND_ELEMENT(data->backends, data->nbackends, backend) < 0) -- 2.21.0
On Mon, Jun 24, 2019 at 05:38:46PM +0200, Peter Krempa wrote:
Move it to qemu_block.c and call it qemuBlockStorageSourceDetachPrepare. It will be reused in other parts as well.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 57 +++++++++++++++++++++++++++++++++++++ src/qemu/qemu_block.h | 4 +++ src/qemu/qemu_hotplug.c | 63 ++--------------------------------------- 3 files changed, 63 insertions(+), 61 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
We use only one copy-on-read filter per disk, so we should handle it separately from the chain. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 46 ++++++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index fe2f577d58..f96209a0bd 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -420,10 +420,6 @@ qemuHotplugRemoveManagedPR(virQEMUDriverPtr driver, struct _qemuHotplugDiskSourceData { qemuBlockStorageSourceAttachDataPtr *backends; size_t nbackends; - - /* disk copy-on-read object */ - virJSONValuePtr corProps; - char *corAlias; }; typedef struct _qemuHotplugDiskSourceData qemuHotplugDiskSourceData; typedef qemuHotplugDiskSourceData *qemuHotplugDiskSourceDataPtr; @@ -437,9 +433,6 @@ qemuHotplugDiskSourceDataFree(qemuHotplugDiskSourceDataPtr data) if (!data) return; - virJSONValueFree(data->corProps); - VIR_FREE(data->corAlias); - for (i = 0; i < data->nbackends; i++) qemuBlockStorageSourceAttachDataFree(data->backends[i]); @@ -453,7 +446,6 @@ qemuHotplugDiskSourceRemovePrepare(virDomainDiskDefPtr disk, virStorageSourcePtr src, virQEMUCapsPtr qemuCaps) { - qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); VIR_AUTOPTR(qemuBlockStorageSourceAttachData) backend = NULL; qemuHotplugDiskSourceDataPtr data = NULL; qemuHotplugDiskSourceDataPtr ret = NULL; @@ -464,9 +456,6 @@ qemuHotplugDiskSourceRemovePrepare(virDomainDiskDefPtr disk, return NULL; if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV)) { - if (VIR_STRDUP(data->corAlias, diskPriv->nodeCopyOnRead) < 0) - goto cleanup; - for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) { if (!(backend = qemuBlockStorageSourceDetachPrepare(n, NULL))) goto cleanup; @@ -517,10 +506,6 @@ qemuHotplugDiskSourceAttachPrepare(virDomainDiskDefPtr disk, return NULL; if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV)) { - if (disk->copy_on_read == VIR_TRISTATE_SWITCH_ON && - !(data->corProps = qemuBlockStorageGetCopyOnReadProps(disk))) - goto cleanup; - for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) { if (!(backend = qemuBlockStorageSourceAttachPrepareBlockdev(n))) goto cleanup; @@ -577,9 +562,6 @@ qemuHotplugDiskSourceAttach(qemuMonitorPtr mon, return -1; } - if (data->corProps && - qemuMonitorAddObject(mon, &data->corProps, &data->corAlias) < 0) - return -1; return 0; } @@ -601,9 +583,6 @@ qemuHotplugDiskSourceRemove(qemuMonitorPtr mon, { size_t i; - if (data->corAlias) - ignore_value(qemuMonitorDelObject(mon, data->corAlias)); - for (i = 0; i < data->nbackends; i++) qemuBlockStorageSourceAttachRollback(mon, data->backends[i]); } @@ -812,6 +791,8 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, qemuHotplugDiskSourceDataPtr diskdata = NULL; char *devstr = NULL; VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); + VIR_AUTOPTR(virJSONValue) corProps = NULL; + VIR_AUTOFREE(char *)corAlias = NULL; if (qemuDomainStorageSourceChainAccessAllow(driver, vm, disk->src) < 0) goto cleanup; @@ -822,6 +803,12 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, if (qemuDomainPrepareDiskSource(disk, priv, cfg) < 0) goto error; + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) { + if (disk->copy_on_read == VIR_TRISTATE_SWITCH_ON && + !(corProps = qemuBlockStorageGetCopyOnReadProps(disk))) + goto cleanup; + } + if (!(diskdata = qemuHotplugDiskSourceAttachPrepare(disk, disk->src, priv->qemuCaps))) goto error; @@ -840,6 +827,10 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, if (qemuHotplugDiskSourceAttach(priv->mon, diskdata) < 0) goto exit_monitor; + if (corProps && + qemuMonitorAddObject(priv->mon, &corProps, &corAlias) < 0) + goto exit_monitor; + if (qemuDomainAttachExtensionDevice(priv->mon, &disk->info) < 0) goto exit_monitor; @@ -865,6 +856,8 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, return ret; exit_monitor: + if (corAlias) + ignore_value(qemuMonitorDelObject(priv->mon, corAlias)); qemuHotplugDiskSourceRemove(priv->mon, diskdata); if (qemuDomainObjExitMonitor(driver, vm) < 0) @@ -4384,10 +4377,13 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainDiskDefPtr disk) { + qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); qemuHotplugDiskSourceDataPtr diskbackend = NULL; virDomainDeviceDef dev; size_t i; qemuDomainObjPrivatePtr priv = vm->privateData; + VIR_AUTOFREE(char *) corAlias = NULL; + bool blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV); int ret = -1; VIR_DEBUG("Removing disk %s from domain %p %s", @@ -4397,6 +4393,11 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, priv->qemuCaps))) return -1; + if (blockdev) { + if (VIR_STRDUP(corAlias, diskPriv->nodeCopyOnRead) < 0) + goto cleanup; + } + for (i = 0; i < vm->def->ndisks; i++) { if (vm->def->disks[i] == disk) { virDomainDiskRemove(vm->def, i); @@ -4406,6 +4407,9 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); + if (corAlias) + ignore_value(qemuMonitorDelObject(priv->mon, corAlias)); + qemuHotplugDiskSourceRemove(priv->mon, diskbackend); if (qemuDomainObjExitMonitor(driver, vm) < 0) -- 2.21.0
On Mon, Jun 24, 2019 at 05:38:47PM +0200, Peter Krempa wrote:
We use only one copy-on-read filter per disk, so we should handle it separately from the chain.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 46 ++++++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 21 deletions(-)
@@ -812,6 +791,8 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, qemuHotplugDiskSourceDataPtr diskdata = NULL; char *devstr = NULL; VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); + VIR_AUTOPTR(virJSONValue) corProps = NULL; + VIR_AUTOFREE(char *)corAlias = NULL;
Space.
if (qemuDomainStorageSourceChainAccessAllow(driver, vm, disk->src) < 0) goto cleanup;
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
These are meant to replace the ad-hoc helpers qemuHotplugDiskSourceAtttach... and the open-coded version in qemu_command.c for use in command line generation. The functions for preparing for attach of chains unfortunately need to be in qemu_command.c as they use function defined by that file and inclusion hierarchy. In this patch new functions are introduced and subsequent patches then refactor individual parts to use them. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 117 ++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_block.h | 29 ++++++++++ src/qemu/qemu_command.c | 67 +++++++++++++++++++++++ src/qemu/qemu_command.h | 11 ++++ 4 files changed, 224 insertions(+) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 560488c852..4130a30e45 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1622,6 +1622,123 @@ qemuBlockStorageSourceDetachPrepare(virStorageSourcePtr src, } +void +qemuBlockStorageSourceChainDataFree(qemuBlockStorageSourceChainDataPtr data) +{ + size_t i; + + if (!data) + return; + + for (i = 0; i < data->nsrcdata; i++) + qemuBlockStorageSourceAttachDataFree(data->srcdata[i]); + + VIR_FREE(data->srcdata); + VIR_FREE(data); +} + + +/** + * qemuBlockStorageSourceChainDetachPrepareBlockdev + * @src: storage source chain to remove + * + * Prepares qemuBlockStorageSourceChainDataPtr for detaching @src and it's + * backingStore if -blockdev was used. + */ +qemuBlockStorageSourceChainDataPtr +qemuBlockStorageSourceChainDetachPrepareBlockdev(virStorageSourcePtr src) +{ + VIR_AUTOPTR(qemuBlockStorageSourceAttachData) backend = NULL; + VIR_AUTOPTR(qemuBlockStorageSourceChainData) data = NULL; + virStorageSourcePtr n; + + if (VIR_ALLOC(data) < 0) + return NULL; + + for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) { + if (!(backend = qemuBlockStorageSourceDetachPrepare(n, NULL))) + return NULL; + + if (VIR_APPEND_ELEMENT(data->srcdata, data->nsrcdata, backend) < 0) + return NULL; + } + + VIR_RETURN_PTR(data); +} + + +/** + * qemuBlockStorageSourceChainDetachPrepareLegacy + * @src: storage source chain to remove + * @driveAlias: Alias of the 'drive' backend (always consumed) + * + * Prepares qemuBlockStorageSourceChainDataPtr for detaching @src and it's + * backingStore if -drive was used. + */ +qemuBlockStorageSourceChainDataPtr +qemuBlockStorageSourceChainDetachPrepareDrive(virStorageSourcePtr src, + char *driveAlias) +{ + VIR_AUTOPTR(qemuBlockStorageSourceAttachData) backend = NULL; + VIR_AUTOPTR(qemuBlockStorageSourceChainData) data = NULL; + + if (VIR_ALLOC(data) < 0) + return NULL; + + if (!(backend = qemuBlockStorageSourceDetachPrepare(src, driveAlias))) + return NULL; + + if (VIR_APPEND_ELEMENT(data->srcdata, data->nsrcdata, backend) < 0) + return NULL; + + VIR_RETURN_PTR(data); +} + + +/** + * qemuBlockStorageSourceChainAttach: + * @mon: monitor object + * @data: storage source chain data + * + * Attach a storage source including it's backing chain and supporting objects. + * Caller must enter @mon prior calling this function. In case of error this + * function returns -1. @data is updated so that qemuBlockStorageSourceChainDetach + * can be used to roll-back the changes. + */ +int +qemuBlockStorageSourceChainAttach(qemuMonitorPtr mon, + qemuBlockStorageSourceChainDataPtr data) +{ + size_t i; + + for (i = data->nsrcdata; i > 0; i--) { + if (qemuBlockStorageSourceAttachApply(mon, data->srcdata[i - 1]) < 0) + return -1; + } + + return 0; +} + + +/** + * qemuBlockStorageSourceChainDetach: + * @mon: monitor object + * @data: storage source chain data + * + * Detach a unused storage source including all it's backing chain and related + * objects described by @data. + */ +void +qemuBlockStorageSourceChainDetach(qemuMonitorPtr mon, + qemuBlockStorageSourceChainDataPtr data) +{ + size_t i; + + for (i = 0; i < data->nsrcdata; i++) + qemuBlockStorageSourceAttachRollback(mon, data->srcdata[i]); +} + + /** * qemuBlockStorageSourceDetachOneBlockdev: * @driver: qemu driver object diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h index a49c73670b..934a1f125d 100644 --- a/src/qemu/qemu_block.h +++ b/src/qemu/qemu_block.h @@ -125,6 +125,35 @@ qemuBlockStorageSourceDetachOneBlockdev(virQEMUDriverPtr driver, qemuDomainAsyncJob asyncJob, virStorageSourcePtr src); +struct _qemuBlockStorageSourceChainData { + qemuBlockStorageSourceAttachDataPtr *srcdata; + size_t nsrcdata; +}; + +typedef struct _qemuBlockStorageSourceChainData qemuBlockStorageSourceChainData; +typedef qemuBlockStorageSourceChainData *qemuBlockStorageSourceChainDataPtr; + +void +qemuBlockStorageSourceChainDataFree(qemuBlockStorageSourceChainDataPtr data); + +qemuBlockStorageSourceChainDataPtr +qemuBlockStorageSourceChainDetachPrepareBlockdev(virStorageSourcePtr src); +qemuBlockStorageSourceChainDataPtr +qemuBlockStorageSourceChainDetachPrepareDrive(virStorageSourcePtr src, + char *driveAlias); + +int +qemuBlockStorageSourceChainAttach(qemuMonitorPtr mon, + qemuBlockStorageSourceChainDataPtr data); + +void +qemuBlockStorageSourceChainDetach(qemuMonitorPtr mon, + qemuBlockStorageSourceChainDataPtr data); + + +VIR_DEFINE_AUTOPTR_FUNC(qemuBlockStorageSourceChainData, + qemuBlockStorageSourceChainDataFree); + int qemuBlockSnapshotAddLegacy(virJSONValuePtr actions, virDomainDiskDefPtr disk, diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b767a1e15f..2a1d22eebf 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -11184,3 +11184,70 @@ qemuBuildStorageSourceAttachPrepareCommon(virStorageSourcePtr src, return 0; } + + +/** + * qemuBuildStorageSourceChainAttachPrepareDrive: + * @disk: disk definition + * @qemuCaps: qemu capabilities object + * + * Prepares qemuBlockStorageSourceChainDataPtr for attaching @disk via -drive. + */ +qemuBlockStorageSourceChainDataPtr +qemuBuildStorageSourceChainAttachPrepareDrive(virDomainDiskDefPtr disk, + virQEMUCapsPtr qemuCaps) +{ + VIR_AUTOPTR(qemuBlockStorageSourceAttachData) elem = NULL; + VIR_AUTOPTR(qemuBlockStorageSourceChainData) data = NULL; + qemuBlockStorageSourceChainDataPtr ret = NULL; + + if (VIR_ALLOC(data) < 0) + return NULL; + + if (!(elem = qemuBuildStorageSourceAttachPrepareDrive(disk, qemuCaps))) + return NULL; + + if (qemuBuildStorageSourceAttachPrepareCommon(disk->src, elem, qemuCaps) < 0) + return NULL; + + if (VIR_APPEND_ELEMENT(data->srcdata, data->nsrcdata, elem) < 0) + return NULL; + + VIR_STEAL_PTR(ret, data); + return ret; +} + + +/** + * qemuBuildStorageSourceChainAttachPrepareDrive: + * @top: storage source chain + * @qemuCaps: qemu capabilities object + * + * Prepares qemuBlockStorageSourceChainDataPtr for attaching @top via -blockdev. + */ +qemuBlockStorageSourceChainDataPtr +qemuBuildStorageSourceChainAttachPrepareBlockdev(virStorageSourcePtr top, + virQEMUCapsPtr qemuCaps) +{ + VIR_AUTOPTR(qemuBlockStorageSourceAttachData) elem = NULL; + VIR_AUTOPTR(qemuBlockStorageSourceChainData) data = NULL; + qemuBlockStorageSourceChainDataPtr ret = NULL; + virStorageSourcePtr n; + + if (VIR_ALLOC(data) < 0) + return NULL; + + for (n = top; virStorageSourceIsBacking(n); n = n->backingStore) { + if (!(elem = qemuBlockStorageSourceAttachPrepareBlockdev(n))) + return NULL; + + if (qemuBuildStorageSourceAttachPrepareCommon(n, elem, qemuCaps) < 0) + return NULL; + + if (VIR_APPEND_ELEMENT(data->srcdata, data->nsrcdata, elem) < 0) + return NULL; + } + + VIR_STEAL_PTR(ret, data); + return ret; +} diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index c885d61578..8695832c16 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -107,6 +107,17 @@ qemuBuildStorageSourceAttachPrepareCommon(virStorageSourcePtr src, qemuBlockStorageSourceAttachDataPtr data, virQEMUCapsPtr qemuCaps); + +qemuBlockStorageSourceChainDataPtr +qemuBuildStorageSourceChainAttachPrepareDrive(virDomainDiskDefPtr disk, + virQEMUCapsPtr qemuCaps); + + +qemuBlockStorageSourceChainDataPtr +qemuBuildStorageSourceChainAttachPrepareBlockdev(virStorageSourcePtr top, + virQEMUCapsPtr qemuCaps); + + char *qemuBuildDiskDeviceStr(const virDomainDef *def, virDomainDiskDefPtr disk, -- 2.21.0
On Mon, Jun 24, 2019 at 05:38:48PM +0200, Peter Krempa wrote:
These are meant to replace the ad-hoc helpers qemuHotplugDiskSourceAtttach...
s/ttt/tt/
and the open-coded version in qemu_command.c for use in command line generation.
The functions for preparing for attach of chains unfortunately need to be in qemu_command.c as they use function defined by that file and
functions? a function?
inclusion hierarchy.
In this patch new functions are introduced and subsequent patches then refactor individual parts to use them.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 117 ++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_block.h | 29 ++++++++++ src/qemu/qemu_command.c | 67 +++++++++++++++++++++++ src/qemu/qemu_command.h | 11 ++++ 4 files changed, 224 insertions(+)
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 560488c852..4130a30e45 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1622,6 +1622,123 @@ qemuBlockStorageSourceDetachPrepare(virStorageSourcePtr src, }
+void +qemuBlockStorageSourceChainDataFree(qemuBlockStorageSourceChainDataPtr data) +{ + size_t i; + + if (!data) + return; + + for (i = 0; i < data->nsrcdata; i++) + qemuBlockStorageSourceAttachDataFree(data->srcdata[i]); + + VIR_FREE(data->srcdata); + VIR_FREE(data); +} + + +/** + * qemuBlockStorageSourceChainDetachPrepareBlockdev + * @src: storage source chain to remove + * + * Prepares qemuBlockStorageSourceChainDataPtr for detaching @src and it's
its
+ * backingStore if -blockdev was used. + */ +qemuBlockStorageSourceChainDataPtr +qemuBlockStorageSourceChainDetachPrepareBlockdev(virStorageSourcePtr src) +{ + VIR_AUTOPTR(qemuBlockStorageSourceAttachData) backend = NULL; + VIR_AUTOPTR(qemuBlockStorageSourceChainData) data = NULL; + virStorageSourcePtr n; + + if (VIR_ALLOC(data) < 0) + return NULL; + + for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) { + if (!(backend = qemuBlockStorageSourceDetachPrepare(n, NULL))) + return NULL; + + if (VIR_APPEND_ELEMENT(data->srcdata, data->nsrcdata, backend) < 0) + return NULL; + } + + VIR_RETURN_PTR(data); +} + + +/** + * qemuBlockStorageSourceChainDetachPrepareLegacy + * @src: storage source chain to remove + * @driveAlias: Alias of the 'drive' backend (always consumed) + * + * Prepares qemuBlockStorageSourceChainDataPtr for detaching @src and it's
its
+ * backingStore if -drive was used. + */ +qemuBlockStorageSourceChainDataPtr +qemuBlockStorageSourceChainDetachPrepareDrive(virStorageSourcePtr src, + char *driveAlias) +{ + VIR_AUTOPTR(qemuBlockStorageSourceAttachData) backend = NULL; + VIR_AUTOPTR(qemuBlockStorageSourceChainData) data = NULL; + + if (VIR_ALLOC(data) < 0) + return NULL; + + if (!(backend = qemuBlockStorageSourceDetachPrepare(src, driveAlias))) + return NULL; + + if (VIR_APPEND_ELEMENT(data->srcdata, data->nsrcdata, backend) < 0) + return NULL; + + VIR_RETURN_PTR(data); +} + + +/** + * qemuBlockStorageSourceChainAttach: + * @mon: monitor object + * @data: storage source chain data + * + * Attach a storage source including it's backing chain and supporting objects.
its
+ * Caller must enter @mon prior calling this function. In case of error this + * function returns -1. @data is updated so that qemuBlockStorageSourceChainDetach + * can be used to roll-back the changes. + */ +int +qemuBlockStorageSourceChainAttach(qemuMonitorPtr mon, + qemuBlockStorageSourceChainDataPtr data) +{ + size_t i; + + for (i = data->nsrcdata; i > 0; i--) { + if (qemuBlockStorageSourceAttachApply(mon, data->srcdata[i - 1]) < 0) + return -1; + } + + return 0; +} + + +/** + * qemuBlockStorageSourceChainDetach: + * @mon: monitor object + * @data: storage source chain data + * + * Detach a unused storage source including all it's backing chain and related
its
+ * objects described by @data. + */ +void +qemuBlockStorageSourceChainDetach(qemuMonitorPtr mon, + qemuBlockStorageSourceChainDataPtr data) +{ + size_t i; + + for (i = 0; i < data->nsrcdata; i++) + qemuBlockStorageSourceAttachRollback(mon, data->srcdata[i]); +} + + /** * qemuBlockStorageSourceDetachOneBlockdev: * @driver: qemu driver object diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h index a49c73670b..934a1f125d 100644 --- a/src/qemu/qemu_block.h +++ b/src/qemu/qemu_block.h @@ -125,6 +125,35 @@ qemuBlockStorageSourceDetachOneBlockdev(virQEMUDriverPtr driver, qemuDomainAsyncJob asyncJob, virStorageSourcePtr src);
+struct _qemuBlockStorageSourceChainData { + qemuBlockStorageSourceAttachDataPtr *srcdata; + size_t nsrcdata; +}; + +typedef struct _qemuBlockStorageSourceChainData qemuBlockStorageSourceChainData; +typedef qemuBlockStorageSourceChainData *qemuBlockStorageSourceChainDataPtr; + +void +qemuBlockStorageSourceChainDataFree(qemuBlockStorageSourceChainDataPtr data); + +qemuBlockStorageSourceChainDataPtr +qemuBlockStorageSourceChainDetachPrepareBlockdev(virStorageSourcePtr src); +qemuBlockStorageSourceChainDataPtr +qemuBlockStorageSourceChainDetachPrepareDrive(virStorageSourcePtr src, + char *driveAlias); + +int +qemuBlockStorageSourceChainAttach(qemuMonitorPtr mon, + qemuBlockStorageSourceChainDataPtr data); + +void +qemuBlockStorageSourceChainDetach(qemuMonitorPtr mon, + qemuBlockStorageSourceChainDataPtr data); + + +VIR_DEFINE_AUTOPTR_FUNC(qemuBlockStorageSourceChainData, + qemuBlockStorageSourceChainDataFree); + int qemuBlockSnapshotAddLegacy(virJSONValuePtr actions, virDomainDiskDefPtr disk, diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b767a1e15f..2a1d22eebf 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -11184,3 +11184,70 @@ qemuBuildStorageSourceAttachPrepareCommon(virStorageSourcePtr src,
return 0; } + + +/** + * qemuBuildStorageSourceChainAttachPrepareDrive: + * @disk: disk definition + * @qemuCaps: qemu capabilities object + * + * Prepares qemuBlockStorageSourceChainDataPtr for attaching @disk via -drive. + */ +qemuBlockStorageSourceChainDataPtr +qemuBuildStorageSourceChainAttachPrepareDrive(virDomainDiskDefPtr disk, + virQEMUCapsPtr qemuCaps) +{ + VIR_AUTOPTR(qemuBlockStorageSourceAttachData) elem = NULL; + VIR_AUTOPTR(qemuBlockStorageSourceChainData) data = NULL; + qemuBlockStorageSourceChainDataPtr ret = NULL; + + if (VIR_ALLOC(data) < 0) + return NULL; + + if (!(elem = qemuBuildStorageSourceAttachPrepareDrive(disk, qemuCaps))) + return NULL; + + if (qemuBuildStorageSourceAttachPrepareCommon(disk->src, elem, qemuCaps) < 0) + return NULL; + + if (VIR_APPEND_ELEMENT(data->srcdata, data->nsrcdata, elem) < 0) + return NULL; + + VIR_STEAL_PTR(ret, data); + return ret;
VIR_RETURN_PTR(data);
+} + + +/** + * qemuBuildStorageSourceChainAttachPrepareDrive: + * @top: storage source chain + * @qemuCaps: qemu capabilities object + * + * Prepares qemuBlockStorageSourceChainDataPtr for attaching @top via -blockdev. + */ +qemuBlockStorageSourceChainDataPtr +qemuBuildStorageSourceChainAttachPrepareBlockdev(virStorageSourcePtr top, + virQEMUCapsPtr qemuCaps) +{ + VIR_AUTOPTR(qemuBlockStorageSourceAttachData) elem = NULL; + VIR_AUTOPTR(qemuBlockStorageSourceChainData) data = NULL; + qemuBlockStorageSourceChainDataPtr ret = NULL; + virStorageSourcePtr n; + + if (VIR_ALLOC(data) < 0) + return NULL; + + for (n = top; virStorageSourceIsBacking(n); n = n->backingStore) { + if (!(elem = qemuBlockStorageSourceAttachPrepareBlockdev(n))) + return NULL; + + if (qemuBuildStorageSourceAttachPrepareCommon(n, elem, qemuCaps) < 0) + return NULL; + + if (VIR_APPEND_ELEMENT(data->srcdata, data->nsrcdata, elem) < 0) + return NULL; + } + + VIR_STEAL_PTR(ret, data); + return ret;
VIR_RETURN_PTR(data); Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Replace the open-coded local implementation with qemuBuildStorageSourceChainAttachPrepare(Drive|Blockdev). Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 34 +++++++--------------------------- 1 file changed, 7 insertions(+), 27 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 2a1d22eebf..acfec8a1bb 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2559,11 +2559,8 @@ qemuBuildDiskSourceCommandLine(virCommandPtr cmd, virDomainDiskDefPtr disk, virQEMUCapsPtr qemuCaps) { - qemuBlockStorageSourceAttachDataPtr *data = NULL; - size_t ndata = 0; - VIR_AUTOPTR(qemuBlockStorageSourceAttachData) tmp = NULL; + VIR_AUTOPTR(qemuBlockStorageSourceChainData) data = NULL; virJSONValuePtr copyOnReadProps = NULL; - virStorageSourcePtr n; char *str = NULL; size_t i; int ret = -1; @@ -2574,35 +2571,21 @@ qemuBuildDiskSourceCommandLine(virCommandPtr cmd, goto cleanup; } - for (n = disk->src; virStorageSourceIsBacking(n); n = n->backingStore) { - if (!(tmp = qemuBlockStorageSourceAttachPrepareBlockdev(n))) - goto cleanup; - - if (qemuBuildStorageSourceAttachPrepareCommon(n, tmp, qemuCaps) < 0) - goto cleanup; - - if (VIR_APPEND_ELEMENT(data, ndata, tmp) < 0) - goto cleanup; - } + if (!(data = qemuBuildStorageSourceChainAttachPrepareBlockdev(disk->src, + qemuCaps))) + goto cleanup; if (disk->copy_on_read == VIR_TRISTATE_SWITCH_ON && !(copyOnReadProps = qemuBlockStorageGetCopyOnReadProps(disk))) goto cleanup; } else { - if (!(tmp = qemuBuildStorageSourceAttachPrepareDrive(disk, qemuCaps))) - goto cleanup; - - if (qemuBuildStorageSourceAttachPrepareCommon(disk->src, tmp, - qemuCaps) < 0) - goto cleanup; - - if (VIR_APPEND_ELEMENT(data, ndata, tmp) < 0) + if (!(data = qemuBuildStorageSourceChainAttachPrepareDrive(disk, qemuCaps))) goto cleanup; } - for (i = ndata; i > 0; i--) { + for (i = data->nsrcdata; i > 0; i--) { if (qemuBuildBlockStorageSourceAttachDataCommandline(cmd, - data[i - 1]) < 0) + data->srcdata[i - 1]) < 0) goto cleanup; } @@ -2617,9 +2600,6 @@ qemuBuildDiskSourceCommandLine(virCommandPtr cmd, ret = 0; cleanup: - for (i = 0; i < ndata; i++) - qemuBlockStorageSourceAttachDataFree(data[i]); - VIR_FREE(data); virJSONValueFree(copyOnReadProps); VIR_FREE(str); return ret; -- 2.21.0
On Mon, Jun 24, 2019 at 05:38:49PM +0200, Peter Krempa wrote:
Replace the open-coded local implementation with qemuBuildStorageSourceChainAttachPrepare(Drive|Blockdev).
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 34 +++++++--------------------------- 1 file changed, 7 insertions(+), 27 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index acfec8a1bb..1ae8a00352 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2560,8 +2560,8 @@ qemuBuildDiskSourceCommandLine(virCommandPtr cmd, virQEMUCapsPtr qemuCaps) { VIR_AUTOPTR(qemuBlockStorageSourceChainData) data = NULL; - virJSONValuePtr copyOnReadProps = NULL; - char *str = NULL; + VIR_AUTOPTR(virJSONValue) copyOnReadProps = NULL; + VIR_AUTOFREE(char *) copyOnReadPropsStr = NULL; size_t i; int ret = -1; @@ -2590,18 +2590,15 @@ qemuBuildDiskSourceCommandLine(virCommandPtr cmd, } if (copyOnReadProps) { - if (!(str = virJSONValueToString(copyOnReadProps, false))) + if (!(copyOnReadPropsStr = virJSONValueToString(copyOnReadProps, false))) goto cleanup; - virCommandAddArgList(cmd, "-blockdev", str, NULL); - VIR_FREE(str); + virCommandAddArgList(cmd, "-blockdev", copyOnReadPropsStr, NULL); } ret = 0; cleanup: - virJSONValueFree(copyOnReadProps); - VIR_FREE(str); return ret; } -- 2.21.0
On Mon, Jun 24, 2019 at 05:38:50PM +0200, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1ae8a00352..7a7497686a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2563,43 +2563,37 @@ qemuBuildDiskSourceCommandLine(virCommandPtr cmd, VIR_AUTOPTR(virJSONValue) copyOnReadProps = NULL; VIR_AUTOFREE(char *) copyOnReadPropsStr = NULL; size_t i; - int ret = -1; if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV)) { - if (virStorageSourceIsEmpty(disk->src)) { - ret = 0; - goto cleanup; - } + if (virStorageSourceIsEmpty(disk->src)) + return 0; if (!(data = qemuBuildStorageSourceChainAttachPrepareBlockdev(disk->src, qemuCaps))) - goto cleanup; + return -1; if (disk->copy_on_read == VIR_TRISTATE_SWITCH_ON && !(copyOnReadProps = qemuBlockStorageGetCopyOnReadProps(disk))) - goto cleanup; + return -1; } else { if (!(data = qemuBuildStorageSourceChainAttachPrepareDrive(disk, qemuCaps))) - goto cleanup; + return -1; } for (i = data->nsrcdata; i > 0; i--) { if (qemuBuildBlockStorageSourceAttachDataCommandline(cmd, data->srcdata[i - 1]) < 0) - goto cleanup; + return -1; } if (copyOnReadProps) { if (!(copyOnReadPropsStr = virJSONValueToString(copyOnReadProps, false))) - goto cleanup; + return -1; virCommandAddArgList(cmd, "-blockdev", copyOnReadPropsStr, NULL); } - ret = 0; - - cleanup: - return ret; + return 0; } -- 2.21.0
On Mon, Jun 24, 2019 at 05:38:51PM +0200, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Replace the use of qemuHotplugDiskSourceAttach* helpers with qemuBuildStorageSourceChainAttachPrepare(Blockdev|Drive). Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index f96209a0bd..413b81c0c7 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -786,9 +786,9 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainDiskDefPtr disk) { + VIR_AUTOPTR(qemuBlockStorageSourceChainData) data = NULL; int ret = -1; qemuDomainObjPrivatePtr priv = vm->privateData; - qemuHotplugDiskSourceDataPtr diskdata = NULL; char *devstr = NULL; VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); VIR_AUTOPTR(virJSONValue) corProps = NULL; @@ -807,11 +807,15 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, if (disk->copy_on_read == VIR_TRISTATE_SWITCH_ON && !(corProps = qemuBlockStorageGetCopyOnReadProps(disk))) goto cleanup; - } - if (!(diskdata = qemuHotplugDiskSourceAttachPrepare(disk, disk->src, - priv->qemuCaps))) - goto error; + if (!(data = qemuBuildStorageSourceChainAttachPrepareBlockdev(disk->src, + priv->qemuCaps))) + goto cleanup; + } else { + if (!(data = qemuBuildStorageSourceChainAttachPrepareDrive(disk, + priv->qemuCaps))) + goto cleanup; + } if (!(devstr = qemuBuildDiskDeviceStr(vm->def, disk, 0, priv->qemuCaps))) goto error; @@ -824,7 +828,7 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); - if (qemuHotplugDiskSourceAttach(priv->mon, diskdata) < 0) + if (qemuBlockStorageSourceChainAttach(priv->mon, data) < 0) goto exit_monitor; if (corProps && @@ -850,7 +854,6 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, ret = 0; cleanup: - qemuHotplugDiskSourceDataFree(diskdata); qemuDomainSecretDiskDestroy(disk); VIR_FREE(devstr); return ret; @@ -858,7 +861,7 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, exit_monitor: if (corAlias) ignore_value(qemuMonitorDelObject(priv->mon, corAlias)); - qemuHotplugDiskSourceRemove(priv->mon, diskdata); + qemuBlockStorageSourceChainDetach(priv->mon, data); if (qemuDomainObjExitMonitor(driver, vm) < 0) ret = -2; -- 2.21.0
On Mon, Jun 24, 2019 at 05:38:52PM +0200, Peter Krempa wrote:
Replace the use of qemuHotplugDiskSourceAttach* helpers with qemuBuildStorageSourceChainAttachPrepare(Blockdev|Drive).
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Use the new helpers for removing the backing chain in case when -blockdev is used. For -drive this function has a local implementation. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 413b81c0c7..1b1d2ba5ce 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -4381,7 +4381,7 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, virDomainDiskDefPtr disk) { qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); - qemuHotplugDiskSourceDataPtr diskbackend = NULL; + VIR_AUTOPTR(qemuBlockStorageSourceChainData) diskBackend = NULL; virDomainDeviceDef dev; size_t i; qemuDomainObjPrivatePtr priv = vm->privateData; @@ -4392,13 +4392,21 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, VIR_DEBUG("Removing disk %s from domain %p %s", disk->info.alias, vm, vm->def->name); - if (!(diskbackend = qemuHotplugDiskSourceRemovePrepare(disk, disk->src, - priv->qemuCaps))) - return -1; if (blockdev) { if (VIR_STRDUP(corAlias, diskPriv->nodeCopyOnRead) < 0) goto cleanup; + + if (!(diskBackend = qemuBlockStorageSourceChainDetachPrepareBlockdev(disk->src))) + goto cleanup; + } else { + char *driveAlias; + + if (!(driveAlias = qemuAliasDiskDriveFromDisk(disk))) + goto cleanup; + + if (!(diskBackend = qemuBlockStorageSourceChainDetachPrepareDrive(disk->src, driveAlias))) + goto cleanup; } for (i = 0; i < vm->def->ndisks; i++) { @@ -4413,7 +4421,7 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, if (corAlias) ignore_value(qemuMonitorDelObject(priv->mon, corAlias)); - qemuHotplugDiskSourceRemove(priv->mon, diskbackend); + qemuBlockStorageSourceChainDetach(priv->mon, diskBackend); if (qemuDomainObjExitMonitor(driver, vm) < 0) goto cleanup; @@ -4435,7 +4443,6 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, ret = 0; cleanup: - qemuHotplugDiskSourceDataFree(diskbackend); virDomainDiskDefFree(disk); return ret; } -- 2.21.0
On Mon, Jun 24, 2019 at 05:38:53PM +0200, Peter Krempa wrote:
Use the new helpers for removing the backing chain in case when -blockdev is used. For -drive this function has a local implementation.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
As this conversion removes the last use of qemuHotplugDiskSource* functions we can remove all of them now. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 188 ++-------------------------------------- 1 file changed, 7 insertions(+), 181 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 1b1d2ba5ce..a1296d55d4 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -417,177 +417,6 @@ qemuHotplugRemoveManagedPR(virQEMUDriverPtr driver, } -struct _qemuHotplugDiskSourceData { - qemuBlockStorageSourceAttachDataPtr *backends; - size_t nbackends; -}; -typedef struct _qemuHotplugDiskSourceData qemuHotplugDiskSourceData; -typedef qemuHotplugDiskSourceData *qemuHotplugDiskSourceDataPtr; - - -static void -qemuHotplugDiskSourceDataFree(qemuHotplugDiskSourceDataPtr data) -{ - size_t i; - - if (!data) - return; - - for (i = 0; i < data->nbackends; i++) - qemuBlockStorageSourceAttachDataFree(data->backends[i]); - - VIR_FREE(data->backends); - VIR_FREE(data); -} - - -static qemuHotplugDiskSourceDataPtr -qemuHotplugDiskSourceRemovePrepare(virDomainDiskDefPtr disk, - virStorageSourcePtr src, - virQEMUCapsPtr qemuCaps) -{ - VIR_AUTOPTR(qemuBlockStorageSourceAttachData) backend = NULL; - qemuHotplugDiskSourceDataPtr data = NULL; - qemuHotplugDiskSourceDataPtr ret = NULL; - char *drivealias = NULL; - virStorageSourcePtr n; - - if (VIR_ALLOC(data) < 0) - return NULL; - - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV)) { - for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) { - if (!(backend = qemuBlockStorageSourceDetachPrepare(n, NULL))) - goto cleanup; - - if (VIR_APPEND_ELEMENT(data->backends, data->nbackends, backend) < 0) - goto cleanup; - } - } else { - if (!(drivealias = qemuAliasDiskDriveFromDisk(disk))) - goto cleanup; - - if (!(backend = qemuBlockStorageSourceDetachPrepare(src, drivealias))) - goto cleanup; - - if (VIR_APPEND_ELEMENT(data->backends, data->nbackends, backend) < 0) - goto cleanup; - } - - VIR_STEAL_PTR(ret, data); - - cleanup: - qemuHotplugDiskSourceDataFree(data); - return ret; -} - - -/** - * qemuHotplugDiskSourceAttachPrepare: - * @disk: disk to generate attachment data for - * @src: disk source to prepare attachment - * @qemuCaps: capabilities of the qemu process - * - * Prepares and returns qemuHotplugDiskSourceData structure filled with all data - * which will fully attach the source backend of the disk to a given VM. - */ -static qemuHotplugDiskSourceDataPtr -qemuHotplugDiskSourceAttachPrepare(virDomainDiskDefPtr disk, - virStorageSourcePtr src, - virQEMUCapsPtr qemuCaps) -{ - VIR_AUTOPTR(qemuBlockStorageSourceAttachData) backend = NULL; - qemuHotplugDiskSourceDataPtr data; - qemuHotplugDiskSourceDataPtr ret = NULL; - virStorageSourcePtr savesrc = NULL; - virStorageSourcePtr n; - - if (VIR_ALLOC(data) < 0) - return NULL; - - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV)) { - for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) { - if (!(backend = qemuBlockStorageSourceAttachPrepareBlockdev(n))) - goto cleanup; - - if (qemuBuildStorageSourceAttachPrepareCommon(n, backend, qemuCaps) < 0) - goto cleanup; - - if (VIR_APPEND_ELEMENT(data->backends, data->nbackends, backend) < 0) - goto cleanup; - } - } else { - VIR_STEAL_PTR(savesrc, disk->src); - disk->src = src; - - if (!(backend = qemuBuildStorageSourceAttachPrepareDrive(disk, qemuCaps))) - goto cleanup; - - VIR_STEAL_PTR(disk->src, savesrc); - - if (qemuBuildStorageSourceAttachPrepareCommon(src, backend, qemuCaps) < 0) - goto cleanup; - - if (VIR_APPEND_ELEMENT(data->backends, data->nbackends, backend) < 0) - goto cleanup; - } - - VIR_STEAL_PTR(ret, data); - - cleanup: - if (savesrc) - VIR_STEAL_PTR(disk->src, savesrc); - - qemuHotplugDiskSourceDataFree(data); - return ret; -} - - -/** - * qemuHotplugDiskSourceAttach: - * @mon: monitor object - * @data: disk backend data object describing what to remove - * - * Attach a disk source backend with all relevant pieces. Caller must enter the - * monitor context for @mon. - */ -static int -qemuHotplugDiskSourceAttach(qemuMonitorPtr mon, - qemuHotplugDiskSourceDataPtr data) -{ - size_t i; - - for (i = data->nbackends; i > 0; i--) { - if (qemuBlockStorageSourceAttachApply(mon, data->backends[i - 1]) < 0) - return -1; - } - - - return 0; -} - - -/** - * qemuHotplugDiskSourceRemove: - * @mon: monitor object - * @data: disk backend data object describing what to remove - * - * Remove a disk source backend with all relevant pieces. This function - * preserves the error which was set prior to calling it. Caller must enter the - * monitor context for @mon. - */ -static void -qemuHotplugDiskSourceRemove(qemuMonitorPtr mon, - qemuHotplugDiskSourceDataPtr data) - -{ - size_t i; - - for (i = 0; i < data->nbackends; i++) - qemuBlockStorageSourceAttachRollback(mon, data->backends[i]); -} - - /** * qemuDomainChangeMediaBlockdev: * @driver: qemu driver structure @@ -614,20 +443,19 @@ qemuDomainChangeMediaBlockdev(virQEMUDriverPtr driver, { qemuDomainObjPrivatePtr priv = vm->privateData; qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); - qemuHotplugDiskSourceDataPtr newbackend = NULL; - qemuHotplugDiskSourceDataPtr oldbackend = NULL; + VIR_AUTOPTR(qemuBlockStorageSourceChainData) newbackend = NULL; + VIR_AUTOPTR(qemuBlockStorageSourceChainData) oldbackend = NULL; char *nodename = NULL; int rc; int ret = -1; if (!virStorageSourceIsEmpty(oldsrc) && - !(oldbackend = qemuHotplugDiskSourceRemovePrepare(disk, oldsrc, - priv->qemuCaps))) + !(oldbackend = qemuBlockStorageSourceChainDetachPrepareBlockdev(oldsrc))) goto cleanup; if (!virStorageSourceIsEmpty(newsrc)) { - if (!(newbackend = qemuHotplugDiskSourceAttachPrepare(disk, newsrc, - priv->qemuCaps))) + if (!(newbackend = qemuBuildStorageSourceChainAttachPrepareBlockdev(newsrc, + priv->qemuCaps))) goto cleanup; if (qemuDomainDiskGetBackendAlias(disk, priv->qemuCaps, &nodename) < 0) @@ -649,11 +477,11 @@ qemuDomainChangeMediaBlockdev(virQEMUDriverPtr driver, rc = qemuMonitorBlockdevMediumRemove(priv->mon, diskPriv->qomName); if (rc == 0 && oldbackend) - qemuHotplugDiskSourceRemove(priv->mon, oldbackend); + qemuBlockStorageSourceChainDetach(priv->mon, oldbackend); if (newbackend && nodename) { if (rc == 0) - rc = qemuHotplugDiskSourceAttach(priv->mon, newbackend); + rc = qemuBlockStorageSourceChainAttach(priv->mon, newbackend); if (rc == 0) rc = qemuMonitorBlockdevMediumInsert(priv->mon, diskPriv->qomName, @@ -669,8 +497,6 @@ qemuDomainChangeMediaBlockdev(virQEMUDriverPtr driver, ret = 0; cleanup: - qemuHotplugDiskSourceDataFree(newbackend); - qemuHotplugDiskSourceDataFree(oldbackend); VIR_FREE(nodename); return ret; } -- 2.21.0
On Mon, Jun 24, 2019 at 05:38:54PM +0200, Peter Krempa wrote:
As this conversion removes the last use of qemuHotplugDiskSource* functions we can remove all of them now.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 188 ++-------------------------------------- 1 file changed, 7 insertions(+), 181 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
[although I'm really not happy with the overprefixing] On Mon, Jun 24, 2019 at 05:38:54PM +0200, Peter Krempa wrote:
As this conversion removes the last use of qemuHotplugDiskSource* functions we can remove all of them now.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 188 ++-------------------------------------- 1 file changed, 7 insertions(+), 181 deletions(-)
Jano
When changing media using blockdev-add we need to remove the leftovers if we didn't succeed plugging in the full chain or closing the tray. Otherwise the data structures will be freed and thus the backing chain members will never be unplugged. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index a1296d55d4..29d703e514 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -491,6 +491,9 @@ qemuDomainChangeMediaBlockdev(virQEMUDriverPtr driver, if (rc == 0) rc = qemuMonitorBlockdevTrayClose(priv->mon, diskPriv->qomName); + if (rc < 0 && newbackend) + qemuBlockStorageSourceChainDetach(priv->mon, newbackend); + if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0) goto cleanup; -- 2.21.0
On Mon, Jun 24, 2019 at 05:38:55PM +0200, Peter Krempa wrote:
When changing media using blockdev-add we need to remove the leftovers if we didn't succeed plugging in the full chain or closing the tray. Otherwise the data structures will be freed and thus the backing chain members will never be unplugged.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 3 +++ 1 file changed, 3 insertions(+)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Peter Krempa