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

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.

> + {.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.

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