On 06/06/2014 05:57 AM, Peter Krempa wrote:
> 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?
blockcopy has the same flag, but yeah, I could go with keep-overlay.
It's shorthand for doing a two-command sequence:
blockcommit --wait
blockjob --abort
but with a nicer name for what the abort actually does in the second phase.
>
>> + },
>> {.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.
Copied from blockcopy; I guess I can fix both instances.
>
> We should discuss the --finish flag name a little bit more.
Is it better to have blockcopy and blockcommit look alike, or to make
each command have a better description of what it is doing?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list