On Thu, Mar 27, 2014 at 05:09:45PM +0800, Fam Zheng wrote:
> Signed-off-by: Fam Zheng <[email protected]>
> ---
> block.c | 30 ++++++++++++++++++++++++++++--
> include/block/block.h | 4 ++++
> 2 files changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/block.c b/block.c
> index 6b82bf0..0abc593 100644
> --- a/block.c
> +++ b/block.c
> @@ -52,6 +52,8 @@
>
> struct BdrvDirtyBitmap {
> HBitmap *bitmap;
> + int64_t size;
> + int64_t granularity;
HBitmap *hbitmap_alloc(uint64_t size, int granularity)
Please use the same types as hbitmap_alloc() for size and granularity.
This way there's less chance of truncation, signedness, or
overflow/underflow problems later. Code becomes messy if types are
inconsistent.
> +void bdrv_reset_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
> +{
> + HBitmap *original = bitmap->bitmap;
> +
> + bitmap->bitmap = hbitmap_alloc(bitmap->size, bitmap->granularity);
> + hbitmap_free(original);
> +}
hbitmap_reset() exists, why allocate a new instance? If speed is a
concern then add a special case to hbitmap_reset() for clearing the
entire bitmap quickly. That way all callers benefit and don't have to
work around the missing functionality like you did here.