Re: [Qemu-devel] [PATCH v13 02/17] qmp: Ensure consistent granularity type

[John Snow <jsnow@redhat.com>]

On 02/16/2015 02:49 PM, Max Reitz wrote:
> On 2015-02-13 at 17:08, John Snow wrote:
>> We treat this field with a variety of different types everywhere
>> in the code. Now it's just uint32_t.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>   block.c                   | 11 ++++++-----
>>   block/mirror.c            |  4 ++--
>>   include/block/block.h     |  2 +-
>>   include/block/block_int.h |  2 +-
>>   qapi/block-core.json      |  2 +-
>>   5 files changed, 11 insertions(+), 10 deletions(-)
>
> Considering you called this series the "$MY_NAME Fixup issue", I'm
> assuming this patch is the response to something I said, maybe that
> Coverity might complain about one right-shift of a 64 bit value with the
> result stored in a 32 bit integer. If so, that would have been
> preventable by assert((x >> 9) <= INT_MAX), so it's not that you would
> have to force everything to be 32 bits wide.
>
> Anyway, I'm fine with either 32 or 64 bits (2 GB maximum granularity
> should be good for now):
>
> Reviewed-by: Max Reitz <mreitz@redhat.com>

You mentioned the types weren't /actually/ consistent, and that did 
actually bother me, so I did try to go out of my way to correct it.

I chose uint32_t to match the existing interface for granularity via the 
drive mirror command, and just whackamole'd the types until they 
flattened out.

Doing it any other way would involve changing other interface options 
elsewhere, and a 2GiB granularity should seriously be sufficient, 
considering we currently generally limit it to under 64KiB :)