[libvirt PATCH 3/3] qemu: block: external snapshot-delete implementation for straightforward cases

Pavel Mores pmores at redhat.com
Tue Mar 31 09:18:29 UTC 2020


Works by blockcommitting the snapshot to be deleted into its parent.  Handles
both deleting children as well and deleting children only (commits the
children into the snapshot referred to by snapshot-delete argument).  If the
necessary block commit operation turns out to be active (the snapshot referred
to by snapshot-delete argument is current, or deleting children is requested)
pivot is performed as well.

This implemetation is limited to the straightforward case which assumes no
branching snapshot chains below the snapshot to be deleted, and the current
snapshot has to be the leaf of the (linear) chain starting at the snapshot to
be deleted.  These requirements are checked and enforced at the top of
qemuDomainSnapshotDeleteExternal().  They might even be too restrictive for
the case where deletion of children is not requested as in that case,
requiring that the parent of the snapshot to be deleted not be a branching
point in snapshot tree should be enough.

The above limitations do not appear to be too severe under the current state
of matters where a major source of branching snapshot structures,
snapshot-revert, is not even implemented for external snapshots yet.  The only
other known cause of branching in external snapshots thus seems to be if a
user creates branching by manually creating snapshot images and metadata and
applies them using snapshot-create --redefine.  At any rate, this work should
be understood as just a first step to a full support of deleting external
snapshots.

Signed-off-by: Pavel Mores <pmores at redhat.com>
---
 src/qemu/qemu_driver.c | 149 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 145 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b981745ecf..4d4f3f069f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16707,6 +16707,149 @@ qemuDomainMomentReparentChildren(void *payload,
 }
 
 
+/*
+ * We can't use virDomainMomentFindLeaf() as it takes a
+ * virDomainMomentObjListPtr which we don't have.  It might be a good idea to
+ * move this function to a library of virDomainMomentObj helpers, then
+ * reimplement virDomainMomentFindLeaf() in terms of this function as it only
+ * uses its virDomainMomentObjListPtr parameter to fish a virDomainMomentObjPtr
+ * out of it.  Then it just runs this function's algorithm on it.
+ */
+static virDomainMomentObjPtr
+myDomainMomentFindLeaf(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 int
+qemuDomainSnapshotDeleteExternal(virDomainObjPtr vm,
+                                 virQEMUDriverPtr driver,
+                                 virDomainMomentObjPtr snap,
+                                 unsigned int flags)
+{
+    int ret = -1;
+    bool isActive;
+    virDomainSnapshotDefPtr snapdef = virDomainSnapshotObjGetDef(snap);
+    virDomainMomentObjPtr leaf = snap->nchildren ? myDomainMomentFindLeaf(snap) : snap;
+    virDomainMomentObjPtr parent = snap->parent;
+    virDomainSnapshotDefPtr parentdef = virDomainSnapshotObjGetDef(parent);
+    size_t i;
+
+    /* This function only works if the chain below 'snap' is linear.  If
+     * there's no unique leaf it means the chain of 'snap's children
+     * branches at some point.  Also, if there *is* a leaf but it's not
+     * the current snapshot, bail out as well. */
+    if (leaf == NULL) {
+        virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
+                       "%s", _("can't delete, snapshot chain branches"));
+        goto cleanup;
+    }
+    if (leaf != virDomainSnapshotGetCurrent(vm->snapshots)) {
+        virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
+                       "%s", _("can't delete, leaf snapshot is not current"));
+        goto cleanup;
+    }
+
+    if (flags & (VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN |
+                 VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY)) {
+        isActive = true;
+    } else {
+        isActive = snap == virDomainSnapshotGetCurrent(vm->snapshots);
+    }
+
+    for (i = 0; i < snapdef->ndisks; i++) {
+        virDomainSnapshotDiskDefPtr snapdisk = &(snapdef->disks[i]);
+        const char *diskName = snapdisk->name;
+        const char *basePath = NULL;
+        const char *topPath = snapdisk->src->path;
+        int blockCommitFlags = VIR_DOMAIN_BLOCK_COMMIT_DELETE;
+        virDomainDiskDefPtr disk;
+
+        if (!(disk = qemuDomainDiskByName(vm->def, diskName)))
+            goto cleanup;
+
+        if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY) {
+            basePath = snapdisk->src->path;
+            topPath = disk->src->path;
+        } else {
+            if (parent->nchildren == 1) {
+                if (parentdef == NULL) {
+                    virStorageSourcePtr baseSrc = virStorageFileChainLookup(disk->src, NULL, NULL, 0, NULL);
+                    if (!baseSrc)
+                        goto cleanup;
+                    basePath = baseSrc->path;
+                } else {
+                    virDomainSnapshotDiskDefPtr parentdisk = &(parentdef->disks[i]);
+                    basePath = parentdisk->src->path;
+                }
+            } else {
+                /* TODO 'snap's parent has multiple children, meaning it's a
+                 * branching point in snapshot tree.  This means we can't
+                 * delete 'snap' by commiting into its parent as doing so would
+                 * corrupt the other branches rooted in the parent.  We might
+                 * still be able to delete 'snap' though by pulling into its
+                 * child/children. */
+
+                virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+                               _("can't delete, parent has multiple children"));
+            }
+
+            if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN)
+                topPath = disk->src->path;
+            else
+                topPath = snapdisk->src->path;
+        }
+
+        if (isActive)
+            blockCommitFlags |= VIR_DOMAIN_BLOCK_COMMIT_ACTIVE;
+
+        if (qemuDomainBlockCommitImpl(vm, driver, diskName, basePath, topPath,
+                                      0, blockCommitFlags) < 0)
+            goto cleanup;
+
+        if (isActive) {
+            /* wait for the blockcommit job to finish (in particular, reach the
+             * QEMU_BLOCKJOB_STATE_READY state)... */
+            qemuBlockJobDataPtr job;
+
+            if (!(job = qemuBlockJobDiskGetJob(disk))) {
+                virReportError(VIR_ERR_INVALID_ARG,
+                            _("disk %s does not have an active block job"), disk->dst);
+                goto cleanup;
+            }
+
+            qemuBlockJobSyncBegin(job);
+            qemuBlockJobUpdate(vm, job, QEMU_ASYNC_JOB_NONE);
+            while (job->state != QEMU_BLOCKJOB_STATE_READY) {
+                if (virDomainObjWait(vm) < 0) {
+                    ret = -1;
+                    goto cleanup;
+                }
+                qemuBlockJobUpdate(vm, job, QEMU_ASYNC_JOB_NONE);
+            }
+            qemuBlockJobSyncEnd(vm, job, QEMU_ASYNC_JOB_NONE);
+
+            /* ... and pivot */
+            if (qemuDomainBlockJobAbortImpl(driver, vm, diskName,
+                                            VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT) < 0)
+                goto cleanup;
+        }
+    }
+
+    ret = 0;
+
+ cleanup:
+    return ret;
+}
+
+
 static int
 qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
                          unsigned int flags)
@@ -16749,10 +16892,8 @@ qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
                                              qemuDomainSnapshotCountExternal,
                                              &external);
         if (external) {
-            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                           _("deletion of %d external disk snapshots not "
-                             "supported yet"), external);
-            goto endjob;
+            if (qemuDomainSnapshotDeleteExternal(vm, driver, snap, flags) < 0)
+                goto endjob;
         }
     }
 
-- 
2.24.1




More information about the libvir-list mailing list