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

[John Snow <jsnow@redhat.com>]

On 11/27/2014 04:41 AM, Max Reitz wrote:
> 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?

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