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-12-01 at 19:52, John Snow wrote:
>
>
> 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.

No need to not use that style if it's just me learning English by false 
(or maybe in 50 years it'll be right) accusations. Thanks for your 
explanation!

Max

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