From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53857) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WTRm5-0006mV-8G for qemu-devel@nongnu.org; Fri, 28 Mar 2014 04:04:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WTRlw-0000uR-6F for qemu-devel@nongnu.org; Fri, 28 Mar 2014 04:04:25 -0400 Received: from mail-we0-x22d.google.com ([2a00:1450:400c:c03::22d]:45076) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WTRlv-0000uL-W1 for qemu-devel@nongnu.org; Fri, 28 Mar 2014 04:04:16 -0400 Received: by mail-we0-f173.google.com with SMTP id w61so2343533wes.18 for ; Fri, 28 Mar 2014 01:04:15 -0700 (PDT) Date: Fri, 28 Mar 2014 09:04:11 +0100 From: Stefan Hajnoczi Message-ID: <20140328080411.GD2441@stefanha-thinkpad.redhat.com> References: <1395911388-31027-1-git-send-email-famz@redhat.com> <1395911388-31027-7-git-send-email-famz@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1395911388-31027-7-git-send-email-famz@redhat.com> Subject: Re: [Qemu-devel] [PATCH v4 6/9] block: Add bdrv_copy_dirty_bitmap and bdrv_reset_dirty_bitmap List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng Cc: Kevin Wolf , Paolo Bonzini , qemu-devel@nongnu.org, Stefan Hajnoczi , Benoit Canet On Thu, Mar 27, 2014 at 05:09:45PM +0800, Fam Zheng wrote: > Signed-off-by: Fam Zheng > --- > 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.