Re: [Qemu-devel] [PATCH RFC for-2.6 0/3] block: Add meta dirty bitmap for migration/persistence

[Fam Zheng <famz@redhat.com>]

On Mon, 12/07 17:19, Vladimir Sementsov-Ogievskiy wrote:
> On 07.12.2015 08:59, Fam Zheng wrote:
> >Vladimir,
> >
> >This is what I propose to implement meta bitmap. It's implemented in the
> >HBitmap level to be more efficient, and the interface slightly varies too.
> 
> What is the benefit?
> 
> Hbitmap usage:
> 
> 1) BdrvDirtyBitmap - need meta
> 2) BackupBlockJob - doesn't need meta
> 3) BlockDirtyBitmapState - doesn't need meta
> 4) now I'm working on series for parallels format and I use HBitmap
> to mark allocated/free clusters.. - doesn't need meta
> 5) your meta hbitmap =) - doesn't need meta..

6) persistence dirty bitmap. - need meta

> 
> So, what is the benefit of moving this functionality to parent
> class? (Which is complicated without it)..

See my reply to John's comment on the cover letter. This is more efficient than
doing it in BdrvDirtyBitmap.

> 
> However, I'm not really against, except my comment to the first patch.
> 
> PS:
> Actually I don't like HBitmap - BdrvDirtyBitmap..
> - No implementation without granularity
>        I need HBitmap without granularity for my needs and have to
> use granularity=0. If there was HBitmap without granularity some
> operations can be faster - for example, finding next/previous/last
> zeros, jumping by words not by bits..
> - It is not sparse. Empty bitmap occupies lots of ram.
> - different granularity units for HBitmap and BdrvDirtyBitmap
> - different layers with/without granularity in hbitmap.c
> - HBitmap with non-zero granularity doesn't know its size (only
> rounded up to granularity)
> - necessity of writing wrappers like
>    bdrv_dirty_bitmap_do_something(...)
>    {
>         hbitmap_do_something(...)
>    }
>    -- Yes, I understand that this is inevitably, but I just don't like it..
> - BdrvDirtyBitmap is defined in block.c.. I think, it should have
> its own .c file.

Yes, I agree we should cut it out during 2.6, with a separate header.