[libvirt] [PATCH v2 06/10] blockcommit: document semantics of committing active layer

[Peter Krempa]

On 06/06/14 00:52, Eric Blake wrote:
> Now that qemu 2.0 allows commit of the active layer, people are
> attempting to use virsh blockcommit and getting into a stuck
> state, because libvirt is unprepared to handle the two-phase
> commit required by qemu.
> 
> Stepping back a bit, there are two valid semantics for a
> commit operation:
> 
> 1. Maintain a 'golden' base, and a transient overlay. Make
> changes in the overlay, and if everything appears to work,
> commit those changes into the base, but still keep the overlay
> for the next round of changes; repeat the cycle as desired.
> 
> 2. Create an external snapshot, then back up the stable state
> in the backing file. Once the backup is complete, commit the
> overlay back into the base, and delete the temporary snapshot.
> 
> Since qemu doesn't know up front which of the two styles is
> preferred, a block commit of the active layer merely gets
> the job into a synchronized state, and sends an event; then
> the user must either cancel (case 1) or complete (case 2),
> where qemu then sends a second event that actually ends the
> job.  However, until commit e6bcbcd, libvirt was blindly
> assuming the semantics that apply to a commit of an
> intermediate image, where there is only one sane conclusion
> (the job automatically ends with fewer elements in the chain);
> and getting stuck because it wasn't prepared for qemu to enter
> a second phase of the job.
> 
> This patch adds a flag to the libvirt API that a user MUST
> supply in order to acknowledge that they will be using two-phase
> semantics.  It might be possible to have a mode where if the
> flag is omitted, we automatically do the case 2 semantics on
> the user's behalf; but before that happens, I must do additional
> patches to track the fact that we are doing an active commit
> in the domain XML.  Later patches will add support of the flag,
> and once 2-phase semantics are working, we can then decide
> whether to relax things to allow an omitted flag to cause an
> automatic pivot.
> 
> * include/libvirt/libvirt.h.in (VIR_DOMAIN_BLOCK_COMMIT_ACTIVE)
> (VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT): New enums.
> * src/libvirt.c (virDomainBlockCommit): Document two-phase job
> when committing active layer, through new flag.
> (virDomainBlockJobAbort): Document that pivot also occurs after
> active commit.
> * tools/virsh-domain.c (vshDomainBlockJob): Cover new job.
> * src/qemu/qemu_driver.c (qemuDomainBlockCommit): Explicitly
> reject active copy; later patches will add it in.
> 
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
>  include/libvirt/libvirt.h.in | 12 ++++++----
>  src/libvirt.c                | 55 +++++++++++++++++++++++++++++---------------
>  src/qemu/qemu_driver.c       | 21 ++++++++++++++---
>  tools/virsh-domain.c         |  3 ++-
>  4 files changed, 65 insertions(+), 26 deletions(-)
> 

> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index b0f80a5..b61b01b 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c

> @@ -15538,9 +15541,21 @@ qemuDomainBlockCommit(virDomainPtr dom,
>       * process; qemu 2.1 is further improving active commit. We need
>       * to start supporting it in libvirt. */
>      if (topSource == disk->src) {
> -        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> -                       _("committing the active layer not supported yet"));
> -        goto endjob;
> +        /* We assume that no one will backport qemu 2.0 active commit
> +         * to an earlier qemu without also backporting the block job
> +         * ready event; but this makes sure of that fact */
> +        if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKJOB_ASYNC)) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                           _("active commit not supported with this QEMU binary"));
> +            goto endjob;
> +        }
> +        /* XXX Should we auto-pivot when COMMIT_ACTIVE is not specified? */

I think we should do it in the future and possibly document the expected
behavior.

> +        if (!(flags & VIR_DOMAIN_BLOCK_COMMIT_ACTIVE)) {
> +            virReportError(VIR_ERR_INVALID_ARG,
> +                           _("commit of '%s' active layer requires active flag"),
> +                           disk->dst);
> +            goto endjob;
> +        }
>      }
> 
>      if (!topSource->backingStore) {

ACK, adding the docs for the auto-pivot commit if you don't specify any
flag can be postponed.

Peter

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 901 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140606/41fabcec/attachment-0001.sig>