[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 6/9] block: Add bdrv_copy_dirty_bitmap and bd
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH v4 6/9] block: Add bdrv_copy_dirty_bitmap and bdrv_reset_dirty_bitmap |
Date: |
Fri, 28 Mar 2014 09:04:11 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Thu, Mar 27, 2014 at 05:09:45PM +0800, Fam Zheng wrote:
> Signed-off-by: Fam Zheng <address@hidden>
> ---
> 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.
- [Qemu-devel] [PATCH v4 1/9] qapi: Add optional field "name" to block dirty bitmap, (continued)
- [Qemu-devel] [PATCH v4 1/9] qapi: Add optional field "name" to block dirty bitmap, Fam Zheng, 2014/03/27
- [Qemu-devel] [PATCH v4 2/9] qmp: Add dirty-bitmap-add and dirty-bitmap-remove, Fam Zheng, 2014/03/27
- [Qemu-devel] [PATCH v4 4/9] block: Introduce bdrv_dirty_bitmap_granularity(), Fam Zheng, 2014/03/27
- [Qemu-devel] [PATCH v4 5/9] hbitmap: Add hbitmap_copy, Fam Zheng, 2014/03/27
- [Qemu-devel] [PATCH v4 6/9] block: Add bdrv_copy_dirty_bitmap and bdrv_reset_dirty_bitmap, Fam Zheng, 2014/03/27
- Re: [Qemu-devel] [PATCH v4 6/9] block: Add bdrv_copy_dirty_bitmap and bdrv_reset_dirty_bitmap,
Stefan Hajnoczi <=
- [Qemu-devel] [PATCH v4 7/9] qmp: Add dirty-bitmap-enable and dirty-bitmap-disable, Fam Zheng, 2014/03/27
- [Qemu-devel] [PATCH v4 8/9] qmp: Add support of "dirty-bitmap" sync mode for drive-backup, Fam Zheng, 2014/03/27
- [Qemu-devel] [PATCH v4 9/9] qapi: Add transaction support to dirty-bitmap-{add, disable}, Fam Zheng, 2014/03/27
- [Qemu-devel] [PATCH v4 3/9] block: Handle error of bdrv_getlength in bdrv_create_dirty_bitmap, Fam Zheng, 2014/03/27