Re: [libvirt PATCH v2 09/10] qemu: block: add external snapshot-delete top-level algorithm

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, May 06, 2020 at 13:42:25 +0200, Pavel Mores wrote:
> This uses the helpers introduced in previous three commits and assembles
> the top-level snapshot deletion algorithm in their terms.
> 
> Note that the third phase - waiting for blockcommits to finish - is
> conceptually optional and a flag/parameter might be exposed to the user in
> the future to skip the third phase.  This would make the whole operation
> asynchronous and let the user deal with concluding the blockcommits
> manually, using the generic blockjob tools.
> 
> Signed-off-by: Pavel Mores <pmores@xxxxxxxxxx>
> ---
>  src/libvirt_private.syms |  1 +
>  src/qemu/qemu_driver.c   | 64 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 65 insertions(+)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 935ef7303b..f68eb500ec 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1049,6 +1049,7 @@ virDomainListCheckpoints;
>  # conf/virdomainmomentobjlist.h
>  virDomainMomentDropChildren;
>  virDomainMomentDropParent;
> +virDomainMomentFindLeaf;

This definitely belongs to a different commit.

>  virDomainMomentForEachChild;
>  virDomainMomentForEachDescendant;
>  virDomainMomentMoveChildren;
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index a2629e9002..57e81e3720 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -17000,6 +17000,70 @@ qemuDomainSnapshotDeleteExternalWaitForJobs(virDomainObjPtr vm,
>  }
>  
>  
> +static int

This is a static function which is unused in this patch. Compilation
will fail.

> +qemuDomainSnapshotDeleteExternal(virDomainObjPtr vm,
> +                                 virQEMUDriverPtr driver,
> +                                 virDomainMomentObjPtr snap,
> +                                 unsigned int flags)
> +{
> +    /* TODO Apr 29, 2020: ultimately, use 'flags' to set this.  Until that
> +     * is supported, just run always synchronously. */
> +    bool async = false;
> +    virDomainSnapshotDefPtr snapdef = virDomainSnapshotObjGetDef(snap);
> +    virDomainMomentObjPtr leaf = snap->nchildren ? virDomainMomentFindLeaf(snap) : snap;
> +    virDomainMomentObjPtr parent = snap->parent;
> +    g_autofree virBlockCommitDesc *blockCommitDescs = NULL;
> +    int numBlockCommits = snapdef->ndisks;
> +
> +    /* 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,
> +                       _("can't delete '%s', snapshot chain branches"),
> +                       snapdef->parent.name);
> +        return -1;
> +    }
> +    if (leaf != virDomainSnapshotGetCurrent(vm->snapshots)) {
> +        virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
> +                       _("can't delete '%s', leaf snapshot is not current"),
> +                       snapdef->parent.name);
> +        return -1;
> +    }
> +    if (parent->nchildren > 1) {
> +        /* 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,
> +                       _("can't delete %s, its parent has multiple children"),
> +                       snapdef->parent.name);
> +        return -1;
> +    }
> +
> +    if (!virDomainObjIsActive(vm))
> +        return -1;
> +
> +    blockCommitDescs = qemuDomainSnapshotDeleteExternalGetJobDescriptors(vm, snap, flags);
> +    if (blockCommitDescs == NULL)
> +        return -1;
> +
> +    if (qemuDomainSnapshotDeleteExternalLaunchJobs(vm, driver, blockCommitDescs, numBlockCommits) < 0)
> +        return -1;
> +
> +    if (!async) {
> +        if (qemuDomainSnapshotDeleteExternalWaitForJobs(vm, driver, blockCommitDescs, numBlockCommits) < 0)
> +            return -1;

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.

> +    }
> +
> +    return 0;
> +}
> +
> +
>  static int
>  qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
>                           unsigned int flags)
> -- 
> 2.24.1
> 




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux