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

[Vladimir Sementsov-Ogievskiy]

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..

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

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.


> I'd like to use these operations to make dirty bitmap persistence more
> efficient too: unchanged dirty bits don't need to be flushed to disk. So I'm
> posting this as a separate series for a common base for both sides.
>
> Posting as RFC as 2.6 dev phase is just starting, we can still tweak the
> interface and/or implementation to fit the need.
>
> Fam Zheng (3):
>    HBitmap: Introduce "meta" bitmap to track bit changes
>    tests: Add test code for meta bitmap
>    block: Support meta dirty bitmap
>
>   block.c                | 46 ++++++++++++++++++++++++++++++-
>   block/mirror.c         |  3 +-
>   blockdev.c             |  3 +-
>   include/block/block.h  | 11 ++++++++
>   include/qemu/hbitmap.h |  7 +++++
>   migration/block.c      |  2 +-
>   tests/test-hbitmap.c   | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++
>   util/hbitmap.c         | 22 +++++++++++++++
>   8 files changed, 164 insertions(+), 4 deletions(-)


--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.