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@xxxxxxxxxx>
> ---
> 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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list