Re: [Qemu-devel] [PATCH v2 4/6] block: add 'speed' optional parameter to block-stream

[Eric Blake]

On 04/24/2012 07:53 AM, Stefan Hajnoczi wrote:
> Allow streaming operations to be started with an initial speed limit.
> This eliminates the window of time between starting streaming and
> issuing block-job-set-speed.  Users should use the new optional 'speed'
> parameter instead so that speed limits are in effect immediately when
> the job starts.
> 

> +++ b/hmp-commands.hx
> @@ -71,8 +71,8 @@ ETEXI
>  
>      {
>          .name       = "block_stream",
> -        .args_type  = "device:B,base:s?",
> -        .params     = "device [base]",
> +        .args_type  = "device:B,speed:o?,base:s?",

Am I remembering correctly that the :o type lets me pass in suffixes,
including 'b' to force bytes, but defaults to 'M' if I don't pass a
suffix?  Should we be documenting the default unit and the ability to
scale as part of the command help (see migrate_set_speed for a similar
command that documents the scaling)?

> +        .params     = "device [speed] [base]",

By having two optional arguments in HMP, can I write 'block_stream
device base' and have things work, or am I now forced to provide a speed
if I want a base?  If the latter, wouldn't this look better as:

.params = "device [speed [base]]",

and if so, how many other HMP commands suffer from the same
documentation flaw?

Thankfully, libvirt doesn't care about HMP for this command (it will
always be using the QMP interface for any qemu new enough to support
block streaming), so I'm not really impacted by the answer to these
questions, other than satisfying my curiosity.  After all, one of the
benefits of QMP is that arguments are in name:value pairs, in part so
that I can provide just the parameters I want in the order I want,
without worrying about providing dummy arguments for any earlier parameters.

-- 
Eric Blake   address@hidden    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature