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.