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@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org