Re: [Qemu-block] [PATCH v2 14/13] block: More operations for meta dirty bitmap

[Vladimir Sementsov-Ogievskiy]

On 26.01.2016 10:26, Fam Zheng wrote:
> Callers can create an iterator of meta bitmap with
> bdrv_dirty_meta_iter_new(), then use the bdrv_dirty_iter_* operations on
> it. Meta iterators are also counted by bitmap->active_iterators.
>
> Also add a couple of functions to retrieve granularity and count.
>
> Signed-off-by: Fam Zheng <address@hidden>
>
> ---
>
> Vladimir, are these interfaces enough for your migration series? If it
> is more convenient to you, you can adjust and take it into your series.

This patch is ok (the only thing - see comment below), thanks. The only 
thing is that my migration series may be changed to use post-copy 
migration instead of meta-bitmaps..

> ---
>   block/dirty-bitmap.c         | 19 +++++++++++++++++++
>   include/block/dirty-bitmap.h |  3 +++
>   2 files changed, 22 insertions(+)
>
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index bf84b17..1aa7f76 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -369,6 +369,11 @@ uint32_t bdrv_dirty_bitmap_granularity(BdrvDirtyBitmap 
> *bitmap)
>       return BDRV_SECTOR_SIZE << hbitmap_granularity(bitmap->bitmap);
>   }
>   
> +uint32_t bdrv_dirty_bitmap_meta_granularity(BdrvDirtyBitmap *bitmap)
> +{
> +    return hbitmap_granularity(bitmap->meta);
> +}

I think it should be more like bdrv_dirty_bitmap_granularity, with 
"BDRV_SECTOR_SIZE << ".

> +
>   BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap *bitmap,
>                                            uint64_t first_sector)
>   {
> @@ -379,6 +384,15 @@ BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap 
> *bitmap,
>       return iter;
>   }
>   
> +BdrvDirtyBitmapIter *bdrv_dirty_meta_iter_new(BdrvDirtyBitmap *bitmap)
> +{
> +    BdrvDirtyBitmapIter *iter = g_new(BdrvDirtyBitmapIter, 1);
> +    hbitmap_iter_init(&iter->hbi, bitmap->meta, 0);
> +    iter->bitmap = bitmap;
> +    bitmap->active_iterators++;
> +    return iter;
> +}
> +
>   void bdrv_dirty_iter_free(BdrvDirtyBitmapIter *iter)
>   {
>       if (!iter) {
> @@ -490,3 +504,8 @@ int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap)
>   {
>       return hbitmap_count(bitmap->bitmap);
>   }
> +
> +int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap)
> +{
> +    return hbitmap_count(bitmap->meta);
> +}
> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
> index 76256fb..d14d923 100644
> --- a/include/block/dirty-bitmap.h
> +++ b/include/block/dirty-bitmap.h
> @@ -30,6 +30,7 @@ void bdrv_enable_dirty_bitmap(BdrvDirtyBitmap *bitmap);
>   BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
>   uint32_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs);
>   uint32_t bdrv_dirty_bitmap_granularity(BdrvDirtyBitmap *bitmap);
> +uint32_t bdrv_dirty_bitmap_meta_granularity(BdrvDirtyBitmap *bitmap);
>   bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap);
>   bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap);
>   const char *bdrv_dirty_bitmap_name(const BdrvDirtyBitmap *bitmap);
> @@ -46,12 +47,14 @@ int bdrv_dirty_bitmap_get_meta(BlockDriverState *bs,
>   void bdrv_dirty_bitmap_reset_meta(BlockDriverState *bs,
>                                     BdrvDirtyBitmap *bitmap, int64_t sector,
>                                     int nb_sectors);
> +BdrvDirtyBitmapIter *bdrv_dirty_meta_iter_new(BdrvDirtyBitmap *bitmap);
>   BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap *bitmap,
>                                            uint64_t first_sector);
>   void bdrv_dirty_iter_free(BdrvDirtyBitmapIter *iter);
>   int64_t bdrv_dirty_iter_next(BdrvDirtyBitmapIter *iter);
>   void bdrv_set_dirty_iter(BdrvDirtyBitmapIter *hbi, int64_t sector_num);
>   int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap);
> +int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap);
>   void bdrv_dirty_bitmap_truncate(BlockDriverState *bs);
>   
>   uint64_t bdrv_dirty_bitmap_serialization_size(const BdrvDirtyBitmap *bitmap,


--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.