So far, only paths could be used to specify blockcommit's base and top images. However, this is not general enough as paths have limitations (most notably they can only work for file-based storage sources). This commit preserves the path-based interface but factors out the core of blockcommit implementation into a separate function that takes its base and top as virStorageSources. The path-based interface basically just converts the paths into virStorageSource just as it was done before and calls the virStorageSource-based implementation core. Signed-off-by: Pavel Mores <pmores(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 108 +++++++++++++++++++++++++---------------- 1 file changed, 65 insertions(+), 43 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b642b24fa2..09300c1e90 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -18441,25 +18441,27 @@ qemuDomainBlockPull(virDomainPtr dom, const char *path, unsigned long bandwidth, } static int -qemuDomainBlockCommitImpl(virDomainObjPtr vm, - virQEMUDriverPtr driver, - const char *path, - const char *base, - const char *top, - unsigned long bandwidth, - unsigned int flags) +qemuDomainBlockCommitCommon(virDomainObjPtr vm, + virQEMUDriverPtr driver, + virDomainDiskDefPtr disk, + virStorageSourcePtr baseSource, + virStorageSourcePtr topSource, + virStorageSourcePtr topParentSource, + unsigned long bandwidth, + unsigned int flags) { qemuDomainObjPrivatePtr priv = vm->privateData; const char *device = NULL; const char *jobname = NULL; int ret = -1; - virDomainDiskDefPtr disk = NULL; - virStorageSourcePtr topSource; - unsigned int topIndex = 0; - virStorageSourcePtr baseSource = NULL; - unsigned int baseIndex = 0; - virStorageSourcePtr top_parent = NULL; bool clean_access = false; + /* TODO the following 2 are just for error reporting. Originally these + * could have been either paths or indexed identifiers (like 'vda[1]'). + * Now the error reporting will always use paths instead of what the + * user originally specified. Find out if this is fine, find a solution + * if it isn't. */ + const char *path = disk->src->path; + const char *base = baseSource->path; g_autofree char *topPath = NULL; g_autofree char *basePath = NULL; g_autofree char *backingPath = NULL; @@ -18473,9 +18475,6 @@ qemuDomainBlockCommitImpl(virDomainObjPtr vm, g_autoptr(virJSONValue) bitmapDisableActions = NULL; VIR_AUTOSTRINGLIST bitmapDisableList = NULL; - if (virDomainObjCheckActive(vm) < 0) - goto endjob; - blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV); /* Convert bandwidth MiB to bytes, if necessary */ @@ -18489,9 +18488,6 @@ qemuDomainBlockCommitImpl(virDomainObjPtr vm, speed <<= 20; } - if (!(disk = qemuDomainDiskByName(vm->def, path))) - goto endjob; - if (virStorageSourceIsEmpty(disk->src)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("disk %s has no source file to be committed"), @@ -18511,14 +18507,6 @@ qemuDomainBlockCommitImpl(virDomainObjPtr vm, goto endjob; } - if (!top || STREQ(top, disk->dst)) - topSource = disk->src; - else if (virStorageFileParseChainIndex(disk->dst, top, &topIndex) < 0 || - !(topSource = virStorageFileChainLookup(disk->src, NULL, - top, topIndex, - &top_parent))) - goto endjob; - if (topSource == disk->src) { if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_ACTIVE_COMMIT)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -18546,13 +18534,6 @@ qemuDomainBlockCommitImpl(virDomainObjPtr vm, goto endjob; } - if (!base && (flags & VIR_DOMAIN_BLOCK_COMMIT_SHALLOW)) - baseSource = topSource->backingStore; - else if (virStorageFileParseChainIndex(disk->dst, base, &baseIndex) < 0 || - !(baseSource = virStorageFileChainLookup(disk->src, topSource, - base, baseIndex, NULL))) - goto endjob; - if ((flags & VIR_DOMAIN_BLOCK_COMMIT_SHALLOW) && baseSource != topSource->backingStore) { virReportError(VIR_ERR_INVALID_ARG, @@ -18580,8 +18561,8 @@ qemuDomainBlockCommitImpl(virDomainObjPtr vm, goto endjob; } - if (blockdev && top_parent && - qemuBlockUpdateRelativeBacking(vm, top_parent, disk->src) < 0) + if (blockdev && topParentSource && + qemuBlockUpdateRelativeBacking(vm, topParentSource, disk->src) < 0) goto endjob; if (virStorageFileGetRelativeBackingPath(topSource, baseSource, @@ -18607,11 +18588,11 @@ qemuDomainBlockCommitImpl(virDomainObjPtr vm, false, false, false) < 0) goto endjob; - if (top_parent && top_parent != disk->src) { - /* While top_parent is topmost image, we don't need to remember its + if (topParentSource && topParentSource != disk->src) { + /* While topParentSource is topmost image, we don't need to remember its * owner as it will be overwritten upon finishing the commit. Hence, * pass chainTop = false. */ - if (qemuDomainStorageSourceAccessAllow(driver, vm, top_parent, + if (qemuDomainStorageSourceAccessAllow(driver, vm, topParentSource, false, false, false) < 0) goto endjob; } @@ -18637,7 +18618,7 @@ qemuDomainBlockCommitImpl(virDomainObjPtr vm, } } - if (!(job = qemuBlockJobDiskNewCommit(vm, disk, top_parent, topSource, + if (!(job = qemuBlockJobDiskNewCommit(vm, disk, topParentSource, topSource, baseSource, &bitmapDisableList, flags & VIR_DOMAIN_BLOCK_COMMIT_DELETE, flags))) @@ -18657,7 +18638,7 @@ qemuDomainBlockCommitImpl(virDomainObjPtr vm, nodetop = topSource->nodeformat; nodebase = baseSource->nodeformat; device = qemuDomainDiskGetTopNodename(disk); - if (!backingPath && top_parent && + if (!backingPath && topParentSource && !(backingPath = qemuBlockGetBackingStoreString(baseSource, false))) goto endjob; @@ -18714,8 +18695,8 @@ qemuDomainBlockCommitImpl(virDomainObjPtr vm, /* Revert access to read-only, if possible. */ qemuDomainStorageSourceAccessAllow(driver, vm, baseSource, true, false, false); - if (top_parent && top_parent != disk->src) - qemuDomainStorageSourceAccessAllow(driver, vm, top_parent, + if (topParentSource && topParentSource != disk->src) + qemuDomainStorageSourceAccessAllow(driver, vm, topParentSource, true, false, false); virErrorRestore(&orig_err); @@ -18725,6 +18706,47 @@ qemuDomainBlockCommitImpl(virDomainObjPtr vm, return ret; } +static int +qemuDomainBlockCommitImpl(virDomainObjPtr vm, + virQEMUDriverPtr driver, + const char *path, + const char *base, + const char *top, + unsigned long bandwidth, + unsigned int flags) +{ + virDomainDiskDefPtr disk = NULL; + virStorageSourcePtr topSource; + unsigned int topIndex = 0; + virStorageSourcePtr baseSource = NULL; + unsigned int baseIndex = 0; + virStorageSourcePtr top_parent = NULL; + + if (virDomainObjCheckActive(vm) < 0) + return -1; + + if (!(disk = qemuDomainDiskByName(vm->def, path))) + return -1; + + if (!top || STREQ(top, disk->dst)) + topSource = disk->src; + else if (virStorageFileParseChainIndex(disk->dst, top, &topIndex) < 0 || + !(topSource = virStorageFileChainLookup(disk->src, NULL, + top, topIndex, + &top_parent))) + return -1; + + if (!base && (flags & VIR_DOMAIN_BLOCK_COMMIT_SHALLOW)) + baseSource = topSource->backingStore; + else if (virStorageFileParseChainIndex(disk->dst, base, &baseIndex) < 0 || + !(baseSource = virStorageFileChainLookup(disk->src, topSource, + base, baseIndex, NULL))) + return -1; + + return qemuDomainBlockCommitCommon(vm, driver, disk, baseSource, topSource, + top_parent, bandwidth, flags); +} + static int qemuDomainBlockCommit(virDomainPtr dom, -- 2.24.1

We'll need the original name for a function that finds leaf for a single virDomainMomentObj. The new name properly advertises that the function actually works on a virDomainMomentObjList, as some (but not all) of existing functions operating on lists do. Signed-off-by: Pavel Mores <pmores(a)redhat.com&gt; --- src/conf/virdomaincheckpointobjlist.c | 2 +- src/conf/virdomainmomentobjlist.c | 2 +- src/conf/virdomainmomentobjlist.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/conf/virdomaincheckpointobjlist.c b/src/conf/virdomaincheckpointobjlist.c index a4942ea706..2df1c426a2 100644 --- a/src/conf/virdomaincheckpointobjlist.c +++ b/src/conf/virdomaincheckpointobjlist.c @@ -186,7 +186,7 @@ virDomainCheckpointUpdateRelations(virDomainCheckpointObjListPtr checkpoints, int ret = virDomainMomentUpdateRelations(checkpoints->base); if (ret == 0) - *leaf = virDomainMomentFindLeaf(checkpoints->base); + *leaf = virDomainMomentObjListFindLeaf(checkpoints->base); return ret; } diff --git a/src/conf/virdomainmomentobjlist.c b/src/conf/virdomainmomentobjlist.c index 18dbd434fb..41e887ba18 100644 --- a/src/conf/virdomainmomentobjlist.c +++ b/src/conf/virdomainmomentobjlist.c @@ -588,7 +588,7 @@ virDomainMomentCheckCycles(virDomainMomentObjListPtr list, /* If there is exactly one leaf node, return that node. */ virDomainMomentObjPtr -virDomainMomentFindLeaf(virDomainMomentObjListPtr list) +virDomainMomentObjListFindLeaf(virDomainMomentObjListPtr list) { virDomainMomentObjPtr moment = &list->metaroot; diff --git a/src/conf/virdomainmomentobjlist.h b/src/conf/virdomainmomentobjlist.h index 75198909ba..cab51edbc4 100644 --- a/src/conf/virdomainmomentobjlist.h +++ b/src/conf/virdomainmomentobjlist.h @@ -121,4 +121,4 @@ int virDomainMomentUpdateRelations(virDomainMomentObjListPtr moments); int virDomainMomentCheckCycles(virDomainMomentObjListPtr list, virDomainMomentDefPtr def, const char *domname); -virDomainMomentObjPtr virDomainMomentFindLeaf(virDomainMomentObjListPtr list); +virDomainMomentObjPtr virDomainMomentObjListFindLeaf(virDomainMomentObjListPtr list); -- 2.24.1

The new function operates on virDomainMomentObjPtr which is indicated by its name. To avoid code duplication, virDomainMomentObjListFindLeaf() was reimplemented in terms of the new function. Signed-off-by: Pavel Mores <pmores(a)redhat.com&gt; --- src/conf/virdomainmomentobjlist.c | 25 +++++++++++++++---------- src/conf/virdomainmomentobjlist.h | 1 + 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/conf/virdomainmomentobjlist.c b/src/conf/virdomainmomentobjlist.c index 41e887ba18..c57d9cc787 100644 --- a/src/conf/virdomainmomentobjlist.c +++ b/src/conf/virdomainmomentobjlist.c @@ -207,6 +207,20 @@ virDomainMomentMoveChildren(virDomainMomentObjPtr from, } +/* If there is exactly one leaf node, return that node. */ +virDomainMomentObjPtr +virDomainMomentFindLeaf(virDomainMomentObjPtr moment) +{ + if (moment->nchildren != 1) + return NULL; + while (moment->nchildren == 1) + moment = moment->first_child; + if (moment->nchildren == 0) + return moment; + return NULL; +} + + static virDomainMomentObjPtr virDomainMomentObjNew(void) { @@ -586,17 +600,8 @@ virDomainMomentCheckCycles(virDomainMomentObjListPtr list, return 0; } -/* If there is exactly one leaf node, return that node. */ virDomainMomentObjPtr virDomainMomentObjListFindLeaf(virDomainMomentObjListPtr list) { - virDomainMomentObjPtr moment = &list->metaroot; - - if (moment->nchildren != 1) - return NULL; - while (moment->nchildren == 1) - moment = moment->first_child; - if (moment->nchildren == 0) - return moment; - return NULL; + return virDomainMomentFindLeaf(&list->metaroot); } diff --git a/src/conf/virdomainmomentobjlist.h b/src/conf/virdomainmomentobjlist.h index cab51edbc4..9c2d01ff8f 100644 --- a/src/conf/virdomainmomentobjlist.h +++ b/src/conf/virdomainmomentobjlist.h @@ -62,6 +62,7 @@ void virDomainMomentMoveChildren(virDomainMomentObjPtr from, virDomainMomentObjPtr to); void virDomainMomentLinkParent(virDomainMomentObjListPtr moments, virDomainMomentObjPtr moment); +virDomainMomentObjPtr virDomainMomentFindLeaf(virDomainMomentObjPtr moment); virDomainMomentObjListPtr virDomainMomentObjListNew(void); void virDomainMomentObjListFree(virDomainMomentObjListPtr moments); -- 2.24.1

Snapshot deletion is implemented as a three-phase process - first we collect specifications of blockcommits used to perform the deletion and run sanity checks to verify that the whole operation is reasonably safe, then we launch the blockcommits and finally we wait for them to finish and check the results. This commit implements the first phase. All information necessary to launch and finish a blockcommit job per disk included in the snapshot is collected in a blockcommit descriptor structure and checked to verify the job makes sense and has a reasonable chance to succeed. Broadly speaking, the checks aim to ensure that the disks in the current running VM are the same as they were when the snapshot was created. Signed-off-by: Pavel Mores <pmores(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 154 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 154 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6ffd53503b..dc1176bd9c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16752,6 +16752,160 @@ qemuDomainMomentReparentChildren(void *payload, } +/* Blockcommit operation descriptor. Holds data required to launch and + * conclude an individual blockcommit job. Generic parameters + * (virDomainObjPtr, virQEMUDriverPtr) are not stored as they are assumed to be + * available at the point of blockcommit invocation. */ + +typedef struct { + virDomainDiskDefPtr disk; + virStorageSourcePtr baseSource; + virStorageSourcePtr topSource; + virStorageSourcePtr topParentSource; + int blockCommitFlags; + bool isActive; /* used to find out if pivot is needed to finish the job */ +} virBlockCommitDesc; + +/* Transforms a snapshot into an array of descriptors of blockcommit jobs + * required to delete the snapshot, one descriptor per affected disk. Also + * runs sanity checks for each affected disk and each individual job to make + * sure the deletion is safe to perform. Returns NULL if safety cannot be + * guaranteed. + * + * (A snapshot is considered safe to delete if, loosely speaking, we can be + * reasonably sure that all disks it touches are still those that were there + * when the snapshot was created. For instance, a different disk might be + * attached to the VM under the same target name, or a disk included in the + * snapshot might be reattached to the VM under a different target name. If + * any such thing happens between the times of snapshot creation and deletion, + * that shapshot would not be considered safe to delete.) */ + +static virBlockCommitDesc * +qemuDomainSnapshotDeleteExternalGetJobDescriptors(virDomainObjPtr vm, + virDomainMomentObjPtr snap, + unsigned int flags) +{ + size_t i; + bool isActive; + virDomainSnapshotDefPtr snapdef = virDomainSnapshotObjGetDef(snap); + virDomainMomentObjPtr parent = snap->parent; + virDomainSnapshotDefPtr parentdef = virDomainSnapshotObjGetDef(parent); + g_autofree virBlockCommitDesc *blockCommitDescs = g_new(virBlockCommitDesc, snapdef->ndisks); + int blockCommitFlags = VIR_DOMAIN_BLOCK_COMMIT_DELETE; + + if (flags & (VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN | + VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY)) { + isActive = true; + } else { + isActive = snap == virDomainSnapshotGetCurrent(vm->snapshots); + } + + if (isActive) + blockCommitFlags |= VIR_DOMAIN_BLOCK_COMMIT_ACTIVE; + + for (i = 0; i < snapdef->ndisks; i++) { + virDomainSnapshotDiskDefPtr snapDisk = &(snapdef->disks[i]); + const char *diskName = snapDisk->name; + virStorageSourcePtr baseSource = NULL; + virStorageSourcePtr topSource = NULL; + virStorageSourcePtr topParentSource = NULL; + virDomainDiskDefPtr domDiskNow; + virDomainDiskDefPtr domDiskThen = virDomainDiskByTarget(snapdef->parent.dom, diskName); + virStorageSourcePtr snapSource = NULL; + virStorageSourcePtr snapParentSource = NULL; + unsigned int snapIndex; + + if (!(domDiskNow = qemuDomainDiskByName(vm->def, diskName))) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("can't delete %s, disk '%s' not found in running VM"), + snapdef->parent.name, diskName); + return NULL; + } + if (snapDisk->src->type != VIR_STORAGE_TYPE_FILE || + domDiskNow->src->type != VIR_STORAGE_TYPE_FILE) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("can't delete %s, only storage type 'file' is supported"), + snapdef->parent.name); + return NULL; + } + + /* TODO Apr 24, 2020: this is not great long-term as it still looks up + * essentially just by path (or target[index]), far from a full + * comparison operator for virStorageSources */ + if (virStorageFileParseChainIndex(domDiskNow->dst, snapDisk->src->path, &snapIndex) < 0 || + !(snapSource = virStorageFileChainLookup(domDiskNow->src, NULL, + snapDisk->src->path, snapIndex, + &snapParentSource))) + return NULL; + + if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY) { + baseSource = snapSource; + } else { + if (parentdef == NULL) { + baseSource = virStorageFileChainLookup(domDiskNow->src, NULL, NULL, 0, NULL); + if (!baseSource) + return NULL; + } else { + baseSource = snapSource->backingStore; + } + } + + if (flags & (VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN | VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY)) { + topSource = domDiskNow->src; + } else { + topSource = snapSource; + topParentSource = snapParentSource; + } + + if (topSource == NULL) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("can't delete %s, couldn't find top (TODO improve this message)"), + snapdef->parent.name); + return NULL; + } + + if (baseSource == NULL) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("can't delete %s, couldn't find base (TODO improve this message)"), + snapdef->parent.name); + return NULL; + } + + /* Check if the disk called 'diskName' is the same as it was at the + * point 'snap' was created. This is more of a heuristic test as + * virStorageSources don't seem to be well equipped for establishing + * identity. The idea is based on the assumption that snapshots can + * only be removed mid-chain, not added. If that holds, the currently + * running chain has to be the same as it was when 'snap' was created + * *minus* the snapshots that were deleted in the meantime, if any. In + * other words, the chain back at the time of 'snap's creation has to + * be an ordered superset of the currently running disk's chain. */ + virStorageSourcePtr chainNow = snapSource->backingStore; + virStorageSourcePtr chainThen = NULL; + for (; virStorageSourceIsBacking(chainNow); chainNow = chainNow->backingStore) { + chainThen = virStorageFileChainLookup(domDiskThen->src, + chainThen, chainNow->path, 0, NULL); + if (!chainThen) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("can't delete %s, disk '%s' doesn't seem the same as when it was created (%s is in %s's backing chain now but it wasn't when the snapshot was created)"), + snapdef->parent.name, diskName, chainNow->path, + snapdef->parent.name); + return NULL; + } + } + + blockCommitDescs[i].disk = domDiskNow; + blockCommitDescs[i].baseSource = baseSource; + blockCommitDescs[i].topSource = topSource; + blockCommitDescs[i].topParentSource = topParentSource; + blockCommitDescs[i].blockCommitFlags = blockCommitFlags; + blockCommitDescs[i].isActive = isActive; + } + + return g_steal_pointer(&blockCommitDescs); +} + + static int qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot, unsigned int flags) -- 2.24.1

This is the second phase of snapshot deletion. We have all information necessary to delete the snapshot by running blockcommits and we haven't detected any problems that would make the deletion unsafe. Now we just launch the blockcommits in parallel. Signed-off-by: Pavel Mores <pmores(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index dc1176bd9c..35b7fb69d5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -154,6 +154,16 @@ static int qemuOpenFileAs(uid_t fallback_uid, gid_t fallback_gid, static virQEMUDriverPtr qemu_driver; +static int +qemuDomainBlockCommitCommon(virDomainObjPtr vm, + virQEMUDriverPtr driver, + virDomainDiskDefPtr disk, + virStorageSourcePtr baseSource, + virStorageSourcePtr topSource, + virStorageSourcePtr topParentSource, + unsigned long bandwidth, + unsigned int flags); + static int qemuDomainBlockCommitImpl(virDomainObjPtr vm, virQEMUDriverPtr driver, @@ -16906,6 +16916,31 @@ qemuDomainSnapshotDeleteExternalGetJobDescriptors(virDomainObjPtr vm, } +static int +qemuDomainSnapshotDeleteExternalLaunchJobs(virDomainObjPtr vm, + virQEMUDriverPtr driver, + const virBlockCommitDesc *blockCommitDescs, + int numDescs) +{ + size_t i; + + for (i = 0; i < numDescs; i++) { + virDomainDiskDefPtr disk = blockCommitDescs[i].disk; + virStorageSourcePtr baseSource = blockCommitDescs[i].baseSource; + virStorageSourcePtr topSource = blockCommitDescs[i].topSource; + virStorageSourcePtr topParentSource = blockCommitDescs[i].topParentSource; + int blockCommitFlags = blockCommitDescs[i].blockCommitFlags; + + if (qemuDomainBlockCommitCommon(vm, driver, disk, baseSource, + topSource, topParentSource, + 0, blockCommitFlags) < 0) + return -1; + } + + return 0; +} + + static int qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot, unsigned int flags) -- 2.24.1

On Wed, May 06, 2020 at 13:42:24 +0200, Pavel Mores wrote: I'm pretty sure that this will not work for more than one disk. If the job for the disk with i=2 finishes sooner than the one for i=1 it will be auto-completed because you set qemuBlockJobSyncBegin(job); after you started the commit. This creates a race condition between this handler and the job itself. All the jobs MUST be marked as 'sync' prior to actually starting them in qemu if you need to wait/handle them here. Aborting the jobs would be wrong IMO. I think that a better option is to wait for all other jobs to complete and report an error. The surrounding code needs to be able to try re-deleting such a snapshot which will then ignore the disks which were already commited as there wouldn't be an recovery path otherwise. The snapshot metadata must stay intact though.

On Wed, May 06, 2020 at 13:42:25 +0200, Pavel Mores wrote: This definitely belongs to a different commit. This is a static function which is unused in this patch. Compilation will fail. This isn't really what I've meant by making it asynchronous. The asyncrhonous job is started by qemuDomainObjBeginAsyncJob and allows other operations to be executed while a long-running API is being executed. In this case we'll need a new async job type for this scenario and this also requires all the monitor calls done here to be converted to take the async job type. The above then allows for the snapshot deletion API to block until the snapshot deletion is done, while other operations such as statistic gathering still work.