Re: [Qemu-devel] [PATCH v8 02/10] qmp: Add block-dirty-bitmap-add and block-dirty-bitmap-remove

[Max Reitz <mreitz@redhat.com>]

On 2014-11-26 at 18:41, John Snow wrote:
> From: Fam Zheng <famz@redhat.com>
>
> The new command pair is added to manage user created dirty bitmap. The
> dirty bitmap's name is mandatory and must be unique for the same device,
> but different devices can have bitmaps with the same names.
>
> The granularity is an optional field. If it is not specified, we will
> choose a default granularity based on the cluster size if available,
> clamped to between 4K and 64K (To mirror how the 'mirror' code was
> already choosing granularity.) If we do not have cluster size info

Maybe swap the right parenthesis and the full stop?

> available, we choose 64K. This code has been factored out into helper

Naturally you're better at English than me, but shouldn't this be "into 
a helper"?

> shared with block/mirror.
>
> The types added to block-core.json will be re-used in future patches
> in this series, see:
> 'qapi: Add transaction support to block-dirty-bitmap-{add, enable, disable}'
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   block.c               | 19 ++++++++++++++++++
>   block/mirror.c        | 10 +---------
>   blockdev.c            | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++
>   include/block/block.h |  1 +
>   qapi/block-core.json  | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++
>   qmp-commands.hx       | 49 +++++++++++++++++++++++++++++++++++++++++++++
>   6 files changed, 179 insertions(+), 9 deletions(-)

Anyway, with or without these minor changes:

Reviewed-by: Max Reitz <mreitz@redhat.com>