Re: [Qemu-devel] [PATCH v2 4/7] dirty-bitmap: separate unused meta-bitmap related functions

[John Snow <jsnow@redhat.com>]

On 04/16/2018 07:44 AM, Vladimir Sementsov-Ogievskiy wrote:
> Separate them in the header and clarify needed locking in comments.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  include/block/dirty-bitmap.h | 14 +++++++++-----
>  block/dirty-bitmap.c         |  5 +++++
>  2 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
> index c7e910016d..b7ccfd1363 100644
> --- a/include/block/dirty-bitmap.h
> +++ b/include/block/dirty-bitmap.h
> @@ -9,9 +9,6 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
>                                            uint32_t granularity,
>                                            const char *name,
>                                            Error **errp);
> -void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap,
> -                                   int chunk_size);
> -void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap);
>  int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,
>                                         BdrvDirtyBitmap *bitmap,
>                                         Error **errp);
> @@ -45,7 +42,6 @@ void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,
>                             int64_t offset, int64_t bytes);
>  void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap,
>                               int64_t offset, int64_t bytes);
> -BdrvDirtyBitmapIter *bdrv_dirty_meta_iter_new(BdrvDirtyBitmap *bitmap);
>  BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap *bitmap);
>  void bdrv_dirty_iter_free(BdrvDirtyBitmapIter *iter);
>  
> @@ -84,7 +80,6 @@ void bdrv_reset_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap,
>  int64_t bdrv_dirty_iter_next(BdrvDirtyBitmapIter *iter);
>  void bdrv_set_dirty_iter(BdrvDirtyBitmapIter *hbi, int64_t offset);
>  int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap);
> -int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap);
>  void bdrv_dirty_bitmap_truncate(BlockDriverState *bs, int64_t bytes);
>  bool bdrv_dirty_bitmap_readonly(const BdrvDirtyBitmap *bitmap);
>  bool bdrv_has_readonly_bitmaps(BlockDriverState *bs);
> @@ -99,4 +94,13 @@ BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap_locked(BlockDriverState *bs,
>                                                    BdrvDirtyBitmap *bitmap,
>                                                    Error **errp);
>  
> +/*
> + * Unused for now meta-bitmaps related functions
> + */
> +

I assume you have plans to use them still? I thought these were useful
for storage migration -- but I guess we don't use these for the postcopy
mechanism?

> +void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap, int chunk_size);
> +void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap);
> +BdrvDirtyBitmapIter *bdrv_dirty_meta_iter_new(BdrvDirtyBitmap *bitmap);
> +int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap);
> +
>  #endif
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index 6c00288fd7..1812e17549 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -149,6 +149,8 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
>   * @bitmap: the block dirty bitmap for which to create a meta dirty bitmap.
>   * @chunk_size: how many bytes of bitmap data does each bit in the meta bitmap
>   * track.
> + *
> + * Called with BQL taken.
>   */
>  void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap,
>                                     int chunk_size)
> @@ -160,6 +162,7 @@ void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap,
>      qemu_mutex_unlock(bitmap->mutex);
>  }
>  
> +/* Called with BQL taken. */
>  void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap)
>  {
>      assert(bitmap->meta);
> @@ -529,6 +532,7 @@ BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap *bitmap)
>      return iter;
>  }
>  
> +/* Called with BQL and dirty_bitmap_mutex locked. */
>  BdrvDirtyBitmapIter *bdrv_dirty_meta_iter_new(BdrvDirtyBitmap *bitmap)
>  {
>      BdrvDirtyBitmapIter *iter = g_new(BdrvDirtyBitmapIter, 1);
> @@ -688,6 +692,7 @@ int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap)
>      return hbitmap_count(bitmap->bitmap);
>  }
>  
> +/* Called with BQL or dirty_bitmap_mutex locked */
>  int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap)
>  {
>      return hbitmap_count(bitmap->meta);
>