qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH v5 3/6] BitmapLog: get the information about the parameters
Date: Tue, 12 Aug 2014 07:53:38 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.7.0

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 <address@hidden>
> ---

...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: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]