Re: [Qemu-devel] [PATCH v6 05/10] block: Add bdrv_copy_dirty_bitmap and bdrv_reset_dirty_bitmap

[Max Reitz]

On 2014-10-30 at 04:22, Fam Zheng wrote:
> Signed-off-by: Fam Zheng <address@hidden>
> ---
>   block.c               | 26 +++++++++++++++++++++++++-
>   include/block/block.h |  4 ++++
>   2 files changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/block.c b/block.c
> index 1b12541..39381cd 100644
> --- a/block.c
> +++ b/block.c
> @@ -53,6 +53,8 @@
>   
>   struct BdrvDirtyBitmap {
>       HBitmap *bitmap;
> +    int64_t size;
> +    int granularity;

Why do you add this field when you never use it except for setting it in 
bdrv_create_dirty_bitmap()? I at least couldn't find any place which 
uses it by quickly scanning the rest of the series...

You could use it for bdrv_dirty_bitmap_granularity(), though.

>       char *name;
>       QLIST_ENTRY(BdrvDirtyBitmap) list;
>   };
> @@ -5280,6 +5282,26 @@ void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, 
> BdrvDirtyBitmap *bitmap)
>       bitmap->name = NULL;
>   }
>   
> +BdrvDirtyBitmap *bdrv_copy_dirty_bitmap(BlockDriverState *bs,
> +                                        const BdrvDirtyBitmap *bitmap,
> +                                        const char *name)
> +{
> +    BdrvDirtyBitmap *new_bitmap;
> +
> +    new_bitmap = g_malloc0(sizeof(BdrvDirtyBitmap));
> +    new_bitmap->bitmap = hbitmap_copy(bitmap->bitmap);
> +    new_bitmap->size = bitmap->size;
> +    new_bitmap->granularity = bitmap->granularity;
> +    new_bitmap->name = name ? g_strdup(name) : NULL;
> +    QLIST_INSERT_HEAD(&bs->dirty_bitmaps, new_bitmap, list);
> +    return new_bitmap;
> +}
> +
> +void bdrv_reset_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
> +{
> +    hbitmap_reset(bitmap->bitmap, 0, bitmap->size);
> +}
> +
>   BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
>                                             int granularity,
>                                             const char *name,
> @@ -5303,7 +5325,9 @@ BdrvDirtyBitmap 
> *bdrv_create_dirty_bitmap(BlockDriverState *bs,
>           return NULL;
>       }
>       bitmap = g_new0(BdrvDirtyBitmap, 1);
> -    bitmap->bitmap = hbitmap_alloc(bitmap_size, ffs(granularity) - 1);
> +    bitmap->size = bitmap_size;
> +    bitmap->granularity = ffs(granularity) - 1;
> +    bitmap->bitmap = hbitmap_alloc(bitmap->size, bitmap->granularity);
>       if (name) {
>           bitmap->name = g_strdup(name);
>       }
> diff --git a/include/block/block.h b/include/block/block.h
> index 87fa48e..c5a3aa6 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -426,6 +426,10 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState 
> *bs,
>   BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs,
>                                           const char *name);
>   void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap 
> *bitmap);
> +void bdrv_reset_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
> +BdrvDirtyBitmap *bdrv_copy_dirty_bitmap(BlockDriverState *bs,
> +                                        const BdrvDirtyBitmap *bitmap,
> +                                        const char *name);
>   void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
>   BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
>   int bdrv_dirty_bitmap_granularity(BlockDriverState *bs,

Reviewed-by: Max Reitz <address@hidden>