Re: [Qemu-devel] [PATCH v4 6/8] BitmapLog: set the frequency of the dump bitmap process

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2630 bytes --]

On 07/17/2014 05:21 AM, Sanidhya Kashyap wrote:
> Rectified the example mistake in qmp-commands.hx.

Is this comment about a mistake that existed prior to your series, or is
it a changelog compared to v1-3 of your posts?  If the former, it might
be nice to say which existing commit was buggy (so we know how long the
docs have been broken); if the latter, then please make it obvious that
it is a patch revision changelog by moving the comment out of the commit
message...

> 
> Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
> ---

...and sticking it here.  Remember, this is the location for comments
that aid review for someone that has looked at earlier revisions of your
series, but which add no value to the qemu.git log a year from now when
only the final version of your series is in place.

>  hmp-commands.hx  | 15 +++++++++++++++
>  hmp.c            | 12 ++++++++++++
>  hmp.h            |  1 +
>  qapi-schema.json | 13 +++++++++++++
>  qmp-commands.hx  | 24 ++++++++++++++++++++++++
>  savevm.c         | 14 ++++++++++++++
>  6 files changed, 79 insertions(+)

Given that your patch is purely additive, I conclude that your commit
comment is in relation to your v1-3 postings of the series, and thus
misplaced.


> +++ b/qapi-schema.json
> @@ -3511,3 +3511,16 @@
>  # Since 2.2
>  ##
>  { 'command': 'log-dirty-bitmap-cancel' }
> +
> +##
> +# @log-dirty-bitmap-set-frequency
> +#
> +# sets the frequency of the dirty bitmap logging process
> +# @frequency: the updated frequency value (in milliseconds).
> +# The min and max values are 10 and 100000 respectively.
> +#
> +# Since 2.2
> +##
> +{ 'command': 'log-dirty-bitmap-set-frequency',
> +  'data': {'frequency': 'int' } }

I hate write-only commands.  Where is the counterpart query command that
I can use to learn if I am currently logging, and what the current
logging frequency is?

> +
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 69d4a07..0a13b74 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -3806,3 +3806,27 @@ Example:
>  <- { "return": {} }
>  
>  EQMP
> +
> +    {
> +        .name       = "log-dirty-bitmap-set-frequency",
> +        .args_type  = "frequency:i",
> +        .mhandler.cmd_new = qmp_marshal_input_log_dirty_bitmap_set_frequency,
> +    },
> +
> +SQMP
> +log-dirty-bitmap-set-frequency
> +--------------------

Nit - the separator line is typically the same length as the command
name, for better visual appeal.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]