Re: [Qemu-devel] [PATCH v11 13/13] block: BdrvDirtyBitmap miscellaneous fixup

[John Snow <jsnow@redhat.com>]

On 01/13/2015 11:50 AM, Vladimir Sementsov-Ogievskiy wrote:
> Hmm. May be, update the size field to be uint64_t too? Negative size is
> not meaningful..
>
> Best regards,
> Vladimir
>

No, it is not meaningful. However, it does match the return type from 
bdrv_nb_sectors(bs), which uses -1 to indicate an error. So we also 
don't really need to change it in this circumstance, since we'll not be 
exceeding what int64_t/bdrv_nb_sectors can provide anyway.

(Plus, INT64_MAX sectors is an absurdly large amount of bytes!)

I'll see how many changes this requires in other helper functions when I 
prepare a V12. If it's easy I will do it.

Thanks!
--js

> On 12.01.2015 19:31, John Snow wrote:
>> (1) granularity type consistency: Update the granularity to be uint64_t
>>      in all places. This value is always in bytes.
>>
>> (2) Some documentation for the fields within BdrvDirtyBitmap.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>   block.c               | 16 ++++++++--------
>>   include/block/block.h |  2 +-
>>   2 files changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index 9e2b8c0..13f9cc0 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -52,13 +52,13 @@
>>   #endif
>>   struct BdrvDirtyBitmap {
>> -    HBitmap *bitmap;
>> -    BdrvDirtyBitmap *successor;
>> -    int64_t size;
>> -    int64_t granularity;
>> -    char *name;
>> -    bool enabled;
>> -    bool frozen;
>> +    HBitmap *bitmap;            /* Dirty sector bitmap */
>> +    BdrvDirtyBitmap *successor; /* Anonymous child */
>> +    int64_t size;               /* Number of sectors */
>> +    uint64_t granularity;       /* Granularity in bytes */
>> +    char *name;                 /* Optional non-empty unique ID */
>> +    bool enabled;               /* Writes are being tracked */
>> +    bool frozen;                /* Protected; see
>> bdrv_freeze_dirty_bitmap() */
>>       QLIST_ENTRY(BdrvDirtyBitmap) list;
>>   };
>> @@ -5350,7 +5350,7 @@ void
>> bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap
>> *bitmap)
>>   }
>>   BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
>> -                                          int granularity,
>> +                                          uint64_t granularity,
>>                                             const char *name,
>>                                             Error **errp)
>>   {
>> diff --git a/include/block/block.h b/include/block/block.h
>> index 99ed679..749429e 100644
>> --- a/include/block/block.h
>> +++ b/include/block/block.h
>> @@ -430,7 +430,7 @@ bool bdrv_qiov_is_aligned(BlockDriverState *bs,
>> QEMUIOVector *qiov);
>>   struct HBitmapIter;
>>   typedef struct BdrvDirtyBitmap BdrvDirtyBitmap;
>>   BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
>> -                                          int granularity,
>> +                                          uint64_t granularity,
>>                                             const char *name,
>>                                             Error **errp);
>>   int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,
>
>

-- 
—js