Re: [Qemu-block] [Qemu-devel] [PATCH v4 17/17] dirty-bitmap: Convert internal hbitmap size/granularity

[John Snow]

On 07/03/2017 11:10 AM, Eric Blake wrote:
> Now that all callers are using byte-based interfaces, there's no
> reason for our internal hbitmap to remain with sector-based
> granularity.  It also simplifies our internal scaling, since we
> already know that hbitmap widens requests out to granularity
> boundaries.
> 
> Signed-off-by: Eric Blake <address@hidden>
> 
> ---
> v4: rebase to earlier changes, include serialization, R-b dropped
> v3: no change
> v2: no change
> ---
>  block/dirty-bitmap.c | 60 
> +++++++++++++++-------------------------------------
>  1 file changed, 17 insertions(+), 43 deletions(-)
> 
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index 5bfc7bc..0b349f0 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -38,7 +38,7 @@
>   */
>  struct BdrvDirtyBitmap {
>      QemuMutex *mutex;
> -    HBitmap *bitmap;            /* Dirty sector bitmap implementation */
> +    HBitmap *bitmap;            /* Dirty bitmap implementation */
>      HBitmap *meta;              /* Meta dirty bitmap */
>      BdrvDirtyBitmap *successor; /* Anonymous child; implies frozen status */
>      char *name;                 /* Optional non-empty unique ID */
> @@ -130,12 +130,7 @@ BdrvDirtyBitmap 
> *bdrv_create_dirty_bitmap(BlockDriverState *bs,
>      }
>      bitmap = g_new0(BdrvDirtyBitmap, 1);
>      bitmap->mutex = &bs->dirty_bitmap_mutex;
> -    /*
> -     * TODO - let hbitmap track full granularity. For now, it is tracking
> -     * only sector granularity, as a shortcut for our iterators.
> -     */
> -    bitmap->bitmap = hbitmap_alloc(DIV_ROUND_UP(bitmap_size, 
> BDRV_SECTOR_SIZE),
> -                                   ctz32(granularity) - BDRV_SECTOR_BITS);
> +    bitmap->bitmap = hbitmap_alloc(bitmap_size, ctz32(granularity));
>      bitmap->size = bitmap_size;
>      bitmap->name = g_strdup(name);
>      bitmap->disabled = false;
> @@ -310,7 +305,6 @@ void bdrv_dirty_bitmap_truncate(BlockDriverState *bs)
>      int64_t size = bdrv_getlength(bs);
> 
>      assert(size >= 0);
> -    size = DIV_ROUND_UP(size, BDRV_SECTOR_SIZE);

whoops uh, were we setting bitmap->size to the *sector* size prior to
this, even though we had already changed bitmap->size to be the byte size?

--js