Re: [Qemu-devel] [PATCH v6 03/10] block: Introduce bdrv_dirty_bitmap_granularity()

[Max Reitz]

On 2014-10-30 at 04:22, Fam Zheng wrote:
This returns the granularity (in sectors) of dirty bitmap.

Actually, it does not.

Signed-off-by: Fam Zheng <f...@redhat.com>
Reviewed-by: Benoit Canet <ben...@irqsave.net>
---
  block.c               | 6 ++++++
  include/block/block.h | 2 ++
  2 files changed, 8 insertions(+)

diff --git a/block.c b/block.c
index dafde4b..1b12541 100644
--- a/block.c
+++ b/block.c
@@ -5356,6 +5356,12 @@ int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap 
*bitmap, int64_t sector
      }
  }
  
+int bdrv_dirty_bitmap_granularity(BlockDriverState *bs,
+                                  BdrvDirtyBitmap *bitmap)
+{
+    return hbitmap_granularity(bitmap->bitmap);
+}
+

hbitmap_granularity() returns bitmap->bitmap->granularity. 
Unfortunately, that value is ffs(granularity >> BDRV_SECTOR_BITS) - 1, 
(= ffs(granularity) - 1 - BDRV_SECTOR_BITS) where "granularity" is the 
original granularity from when the dirty bitmap was created. Therefore, 
this should be 1 << hbitmap_granularity(bitmap->bitmap) to be the 
granularity in sectors.

See bdrv_query_dirty_bitmaps() (written by you :-)).

And frankly, I'd prefer this not to return the granularity in sectors 
but in bytes, because that's the unit the granularity is specified in 
when the dirty bitmap is created; also, as far as my understanding goes, 
we want to get away from sectors, so I wouldn't introduce a new function 
using that unit if there's no real need for it.

Max

  void bdrv_dirty_iter_init(BlockDriverState *bs,
                            BdrvDirtyBitmap *bitmap, HBitmapIter *hbi)
  {
diff --git a/include/block/block.h b/include/block/block.h
index b0ef82c..87fa48e 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -428,6 +428,8 @@ BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState 
*bs,
  void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap 
*bitmap);
  void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
  BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
+int bdrv_dirty_bitmap_granularity(BlockDriverState *bs,
+                                  BdrvDirtyBitmap *bitmap);
  int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t 
sector);
  void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector, int nr_sectors);
  void bdrv_reset_dirty(BlockDriverState *bs, int64_t cur_sector, int 
nr_sectors);