Re: [Qemu-devel] [PATCH v5 3/6] BitmapLog: get the information about the parameters

[Eric Blake <eblake@redhat.com>]

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

On 07/31/2014 09:12 PM, Sanidhya Kashyap wrote:
> No functional change except the variable name.

This comment feels more like it is a changelog of what is different from
v4.  If so, it belongs...

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

...here, after the --- separator.  It makes no sense in isolation in
qemu.git (where v1 through v4 do not appear), but is there only to aid
reviewers on list (who DO see prior versions, and want to see if you
took into account earlier review comments).


> +    if (info) {
> +        monitor_printf(mon, "current iteration: %ld\n",
> +                       info->current_iteration);

Won't compile on 32-bit.  Per patch 2/6, info->current_iteration is
int64_t, but %ld might be 32-bit.  Furthermore, patch 2/6 had an
(arbitrary?) limit of 100,000 as the maximum iteration request, which
fits in a 32-bit value to begin with, so using int64_t to hold the value
is overkill.


> +Example:
> +
> +-> { "execute": "query-log-dirty-bitmap" }
> +<- { "return": {
> +            "current-iteration": 3
> +            "iterations": 10
> +            "period": 100 } }

That's not valid JSON.  You are missing two commas.  It's best to paste
an actual QMP result, rather than trying to write it by hand.

-- 
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: 539 bytes --]