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

[John Snow]

On 11/27/2014 04:41 AM, Max Reitz wrote:
> On 2014-11-26 at 18:41, John Snow wrote:
> > From: Fam Zheng <address@hidden>
> >
> > 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?

This is an American thing, the difference between "aesthetic 
punctuation" and "logical punctuation." (<-- aesthetic.)

http://www.slate.com/articles/life/the_good_word/2011/05/the_rise_of_logical_punctuation.html

I can make a mental note in the future to not use the American style, I 
just thought it would be fun to explain it.

> > 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"?

This, on the other hand, is just a typo where my brain filled in the 
missing glue for me.

> > 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 <address@hidden>
> > Signed-off-by: John Snow <address@hidden>
> > ---
> >   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 <address@hidden>