On 09/17/2012 05:29 PM, Peter Krempa wrote:
> On 09/18/12 00:08, Eric Blake wrote:
>> The wait loop logic borrows heavily from blockcommit.
I _meant_ to say 'blockpull'. And because of my cryptic reference,
instead of a proper attribution,
>
> Maybe a little too sparse on the commit message, but the man page
> addition makes it up.
...I understand why you had a harder time reviewing it.
>
>> + {"bandwidth", VSH_OT_DATA, VSH_OFLAG_NONE, N_("bandwidth limit in
>> MiB/s")},
>
> Megabits look like a reasonable granularity to limit this. Well maybe
> apart from testing purposes where somebody would like to do it really
> slow to be able to poke around.
That, and the existing 'virsh blockjob --bandwidth' command sets MiB/s,
so we really don't have a choice in our units since this is just another
case of a block job.
>> + } else if (verbose || vshCommandOptBool(cmd, "timeout") ||
>> + vshCommandOptBool(cmd, "async")) {
>> + vshError(ctl, "%s", _("blocking control options require
>> --wait"));
>
> This error message isn't 100% clear on the first read. What about:
> "--verbose and --timeout require --wait" ?
Sure. Again, this was copied from 'virsh blockpull', so I'll make the
same change there.
>> + GETTIMEOFDAY(&curr);
>> + if (intCaught || (timeout &&
>> + (((int)(curr.tv_sec - start.tv_sec) * 1000 +
>> + (int)(curr.tv_usec - start.tv_usec) /
>> 1000) >
>> + timeout * 1000))) {
>
> Wouldn't it be better to multiply timeout by 1000 at the beginning and
> not bother re-doing it on every iteration?
Copy and paste strikes again :)
>> + vshPrint(ctl, "\n%s", quit ? _("Commit aborted") : _("Commit
>> complete"));
>
> Meh a ternary ... but saves lines.
and again.
>
> Your docs are always great. ACK
I'll fix the nits, and push this shortly, then.
--
Eric Blake eblake@xxxxxxxxxx +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