[libvirt] [PATCH v2 07/10] virsh: expose new active commit controls

[Peter Krempa]

On 06/06/14 00:52, Eric Blake wrote:
> Add knobs to virsh to manage a 2-phase active commit of the top
> layer, similar to knobs already present on blockcopy.  While this
> code will fail until later patches actually implement the new
> knobs in the qemu driver, doing it now proves that the API is
> usable and also makes it easier for testing the qemu changes as
> they are made.
> 
> * tools/virsh-domain.c (cmdBlockCommit): Add --active, --pivot,
> and --finish options, modeled after blockcopy.
> (blockJobImpl): Support --active flag.
> * tools/virsh.pod (blockcommit): Document new flags.
> (blockjob): Mention 2-phase commit interaction.
> 
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
>  tools/virsh-domain.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++--
>  tools/virsh.pod      | 27 +++++++++++++++++++-------
>  2 files changed, 72 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 6da8a17..a879316 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c

> @@ -1608,6 +1615,14 @@ static const vshCmdOptDef opts_block_commit[] = {
>       .type = VSH_OT_INT,
>       .help = N_("with --wait, abort if copy exceeds timeout (in seconds)")
>      },
> +    {.name = "pivot",
> +     .type = VSH_OT_BOOL,
> +     .help = N_("with --active --wait, pivot when commit is synced")
> +    },
> +    {.name = "finish",
> +     .type = VSH_OT_BOOL,
> +     .help = N_("with --active --wait, quit when commit is synced")

Finish seems a bit too generic ... shouldn't we go with something like
--keep-overlay or other more specific flag?

> +    },
>      {.name = "async",
>       .type = VSH_OT_BOOL,
>       .help = N_("with --wait, don't wait for cancel to finish")

> @@ -1709,7 +1744,22 @@ cmdBlockCommit(vshControl *ctl, const vshCmd *cmd)
>          /* printf [100 %] */
>          vshPrintJobProgress(_("Block Commit"), 0, 1);
>      }
> -    vshPrint(ctl, "\n%s", quit ? _("Commit aborted") : _("Commit complete"));
> +    if (!quit && pivot) {
> +        abort_flags |= VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT;
> +        if (virDomainBlockJobAbort(dom, path, abort_flags) < 0) {
> +            vshError(ctl, _("failed to pivot job for disk %s"), path);
> +            goto cleanup;
> +        }
> +    } else if (finish && !quit &&
> +               virDomainBlockJobAbort(dom, path, abort_flags) < 0) {
> +        vshError(ctl, _("failed to finish job for disk %s"), path);
> +        goto cleanup;
> +    }
> +    vshPrint(ctl, "\n%s",
> +             quit ? _("Commit aborted") :
> +             pivot ? _("Successfully pivoted") :
> +             !finish ? _("Now in synchronized phase") :
> +             _("Commit complete"));

Uhhh, embedded ternaries? Please change it to hierarchical if's or
something.

> 
>      ret = true;
>   cleanup:


> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index 02671b4..fc81fac 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -767,8 +767,9 @@ address of virtual interface (such as I<detach-interface> or
>  I<domif-setlink>) will accept the MAC address printed by this command.
> 
>  =item B<blockcommit> I<domain> I<path> [I<bandwidth>]
> -{[I<base>] | [I<--shallow>]} [I<top>] [I<--delete>]
> -[I<--wait> [I<--verbose>] [I<--timeout> B<seconds>] [I<--async>]]
> +{[I<base>] | [I<--shallow>]} [I<top>] [I<--delete>] [I<--active>]
> +[I<--wait> [I<--verbose>] [{I<--pivot> | I<--finish>}]
> +[I<--timeout> B<seconds>] [I<--async>]]
> 
>  Reduce the length of a backing image chain, by committing changes at the
>  top of the chain (snapshot or delta files) into backing images.  By
> @@ -778,8 +779,17 @@ operation is constrained to committing just that portion of the chain;
>  I<--shallow> can be used instead of I<base> to specify the immediate
>  backing file of the resulting top image to be committed.  The files
>  being committed are rendered invalid, possibly as soon as the operation
> -starts; using the I<--delete> flag will remove these files at the successful
> -completion of the commit operation.
> +starts; using the I<--delete> flag will attempt to remove these invalidated
> +files at the successful completion of the commit operation.
> +
> +When I<top> is omitted or specified as the active image, it is also
> +possible to specify I<--active> to trigger a two-phase active commit. In
> +the first phase, I<top> is copied into I<base> and the job can only be
> +canceled, with top still containing data not yet in base. In the second
> +phase, I<top> and I<base> remain identical until a call to B<blockjob>
> +with the I<--abort> flag (keeping top as the active image that tracks
> +changes from that point in time) or the I<--pivot> flag (making base
> +the new active image and invalidating top).
> 
>  By default, this command returns as soon as possible, and data for
>  the entire disk is committed in the background; the progress of the
> @@ -790,7 +800,10 @@ or SIGINT is sent (usually with C<Ctrl-C>).  Using I<--verbose> along
>  with I<--wait> will produce periodic status updates.  If job cancellation
>  is triggered, I<--async> will return control to the user as fast as
>  possible, otherwise the command may continue to block a little while
> -longer until the job is done cleaning up.
> +longer until the job is done cleaning up.  When I<--active> is used
> +alongside I<--wait>, then it is also possible to use I<--pivot> or
> +I<--finish> to end the job cleanly rather than leaving things in the

I'd drop cleanly. The synchronized state is "clean" too imho.

> +synchronized state.
> 
>  I<path> specifies fully-qualified path of the disk; it corresponds
>  to a unique target name (<target dev='name'/>) or source file (<source
> @@ -916,8 +929,8 @@ also B<domblklist> for listing these names).
>  If I<--abort> is specified, the active job on the specified disk will
>  be aborted.  If I<--async> is also specified, this command will return
>  immediately, rather than waiting for the cancellation to complete.  If
> -I<--pivot> is specified, this requests that an active copy job
> -be pivoted over to the new copy.
> +I<--pivot> is specified, this requests that an active copy or active
> +commit job be pivoted over to the new image.
>  If I<--info> is specified, the active job information on the specified
>  disk will be printed.
>  I<bandwidth> can be used to set bandwidth limit for the active job.
> 

We should discuss the --finish flag name a little bit more.

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/82306d0b/attachment-0001.sig>