diff mbox

[v11,13/13] block: BdrvDirtyBitmap miscellaneous fixup

Message ID 1421080265-2228-14-git-send-email-jsnow@redhat.com
State New
Headers show

Commit Message

John Snow Jan. 12, 2015, 4:31 p.m. UTC
(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(-)

Comments

Vladimir Sementsov-Ogievskiy Jan. 13, 2015, 4:50 p.m. UTC | #1
Hmm. May be, update the size field to be uint64_t too? Negative size is 
not meaningful..

Best regards,
Vladimir

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,
John Snow Jan. 13, 2015, 6:27 p.m. UTC | #2
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,
>
>
diff mbox

Patch

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,