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

[Eric Blake]

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

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