From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37585) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y5x9Q-0001vt-AM for qemu-devel@nongnu.org; Tue, 30 Dec 2014 08:47:57 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Y5x9M-0004En-7x for qemu-devel@nongnu.org; Tue, 30 Dec 2014 08:47:56 -0500 Received: from mx2.parallels.com ([199.115.105.18]:59902) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y5x9L-0004Ef-VW for qemu-devel@nongnu.org; Tue, 30 Dec 2014 08:47:52 -0500 Message-ID: <54A2ACF7.2020302@parallels.com> Date: Tue, 30 Dec 2014 16:47:35 +0300 From: Vladimir Sementsov-Ogievskiy MIME-Version: 1.0 References: <1419297142-24282-1-git-send-email-jsnow@redhat.com> <1419297142-24282-7-git-send-email-jsnow@redhat.com> In-Reply-To: <1419297142-24282-7-git-send-email-jsnow@redhat.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v10 06/13] block: Add bdrv_copy_dirty_bitmap and bdrv_clear_dirty_bitmap List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: John Snow , qemu-devel@nongnu.org Cc: kwolf@redhat.com, famz@redhat.com, armbru@redhat.com, mreitz@redhat.com, stefanha@redhat.com I'm sorry if it was already discussed, but I think it is inconsistent to have "size" in sectors and "granularity" in bytes in one structure. I've misused these fields because of this in my current work. At least, I think there should be comments about this. Best regards, Vladimir On 23.12.2014 04:12, John Snow wrote: > Signed-off-by: Fam Zheng > Signed-off-by: John Snow > --- > block.c | 39 +++++++++++++++++++++++++++++++++++---- > include/block/block.h | 4 ++++ > 2 files changed, 39 insertions(+), 4 deletions(-) > > diff --git a/block.c b/block.c > index a1d9e88..f9e0767 100644 > --- a/block.c > +++ b/block.c > @@ -53,6 +53,8 @@ > > struct BdrvDirtyBitmap { > HBitmap *bitmap; > + int64_t size; > + int64_t granularity; > char *name; > QLIST_ENTRY(BdrvDirtyBitmap) list; > }; > @@ -5343,6 +5345,21 @@ void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap *bitmap) > bitmap->name = NULL; > } > > +BdrvDirtyBitmap *bdrv_copy_dirty_bitmap(BlockDriverState *bs, > + BdrvDirtyBitmap *bitmap, > + const char *name) > +{ > + BdrvDirtyBitmap *new_bitmap; > + > + new_bitmap = g_malloc0(sizeof(BdrvDirtyBitmap)); > + new_bitmap->bitmap = hbitmap_copy(bitmap->bitmap); > + new_bitmap->size = bitmap->size; > + new_bitmap->granularity = bitmap->granularity; > + new_bitmap->name = g_strdup(name); > + QLIST_INSERT_HEAD(&bs->dirty_bitmaps, new_bitmap, list); > + return new_bitmap; > +} > + > BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, > int granularity, > const char *name, > @@ -5350,6 +5367,7 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, > { > int64_t bitmap_size; > BdrvDirtyBitmap *bitmap; > + int sector_granularity; > > assert((granularity & (granularity - 1)) == 0); > > @@ -5357,8 +5375,8 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, > error_setg(errp, "Bitmap already exists: %s", name); > return NULL; > } > - granularity >>= BDRV_SECTOR_BITS; > - assert(granularity); > + sector_granularity = granularity >> BDRV_SECTOR_BITS; > + assert(sector_granularity); > bitmap_size = bdrv_nb_sectors(bs); > if (bitmap_size < 0) { > error_setg_errno(errp, -bitmap_size, "could not get length of device"); > @@ -5366,7 +5384,9 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, > return NULL; > } > bitmap = g_new0(BdrvDirtyBitmap, 1); > - bitmap->bitmap = hbitmap_alloc(bitmap_size, ffs(granularity) - 1); > + bitmap->size = bitmap_size; > + bitmap->granularity = granularity; > + bitmap->bitmap = hbitmap_alloc(bitmap->size, ffs(sector_granularity) - 1); > bitmap->name = g_strdup(name); > QLIST_INSERT_HEAD(&bs->dirty_bitmaps, bitmap, list); > return bitmap; > @@ -5439,7 +5459,9 @@ uint64_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs) > uint64_t bdrv_dirty_bitmap_granularity(BlockDriverState *bs, > BdrvDirtyBitmap *bitmap) > { > - return BDRV_SECTOR_SIZE << hbitmap_granularity(bitmap->bitmap); > + g_assert(BDRV_SECTOR_SIZE << hbitmap_granularity(bitmap->bitmap) == > + bitmap->granularity); > + return bitmap->granularity; > } > > void bdrv_dirty_iter_init(BlockDriverState *bs, > @@ -5460,6 +5482,15 @@ void bdrv_reset_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, > hbitmap_reset(bitmap->bitmap, cur_sector, nr_sectors); > } > > +/** > + * Effectively, reset the hbitmap from bits [0, size) > + * Synonymous with bdrv_reset_dirty_bitmap(bs, bitmap, 0, bitmap->size) > + */ > +void bdrv_clear_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap) > +{ > + hbitmap_reset(bitmap->bitmap, 0, bitmap->size); > +} > + > static void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector, > int nr_sectors) > { > diff --git a/include/block/block.h b/include/block/block.h > index c7402e7..e964abd 100644 > --- a/include/block/block.h > +++ b/include/block/block.h > @@ -436,6 +436,9 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, > BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs, > const char *name); > void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap *bitmap); > +BdrvDirtyBitmap *bdrv_copy_dirty_bitmap(BlockDriverState *bs, > + BdrvDirtyBitmap *bitmap, > + const char *name); > void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap); > BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs); > uint64_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs); > @@ -446,6 +449,7 @@ void bdrv_set_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, > int64_t cur_sector, int nr_sectors); > void bdrv_reset_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, > int64_t cur_sector, int nr_sectors); > +void bdrv_clear_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap); > void bdrv_dirty_iter_init(BlockDriverState *bs, > BdrvDirtyBitmap *bitmap, struct HBitmapIter *hbi); > int64_t bdrv_get_dirty_count(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);