[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH for 2.6 2/3] block: Hide HBitmap in
Re: [Qemu-block] [Qemu-devel] [PATCH for 2.6 2/3] block: Hide HBitmap in block dirty bitmap interface
Tue, 24 Nov 2015 14:19:36 -0500
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0
On 11/23/2015 09:28 PM, Fam Zheng wrote:
> On Mon, 11/23 16:34, John Snow wrote:
>> Hmm, what's the idea, here?
>> This patch does a lot more than just hide hbitmap details from callers
>> of block_dirty_bitmap functions.
>> So we're changing the backing hbitmap to always be one where g=0 and the
>> number of physical bits directly is (now) the same as the number of
>> 'virtual' bits, pre-patch. Then, to compensate, we handle the shift math
>> to convert the bitmap granularity to sector size and vice-versa in the
>> Block Dirty Bitmap layer instead of in the hbitmap layer.
>> What's the benefit? It looks like we just pull all the implementation
>> details up from hbitmap and into BdrvDirtyBitmap, which I am not
>> immediately convinced of as being a benefit.
> It feels counter intuitive to me with hbitmap handling granularity, it makes
> more like a HGranularityBitmap rather than HBitmap, and is unnecessarily
> complex to work on.
I guess it's a matter of personal taste on where to try to hide the
complexity. Since hbitmap can and already does manage it for us, inertia
leaves me satisfied with this option.
I don't know if pulling the granularity out of hbitmap and into
BdrvDirtyBitmap (so now we have granularity management code in two
objects) is a significant gain, but I wouldn't NACK this over that
specifically ... I'll let Vladimir et al decide if this does make e.g.
his migration/persistence patches easier to write or not.
I agree that the new iterator object for BdrvDirtyBitmap is good,
though, and wouldn't mind seeing this split up into its two parts:
(1) Hiding the hbitmap implementation detail entirely from users of
BdrvDirtyBitmap, by adding new BdrvDirtyBitmap iterators
(2) Moving the granularity logic up into BdrvDirtyBitmap
> Now it's simplified in that only one BdrvDirtyBitmap needs to care about the
> granularity, and which I think is a big benefit when we are going to extend
> dirty bitmap interface, for example to serialize and deserialize for
[Qemu-block] [PATCH for 2.6 3/3] hbitmap: Drop "granularity", Fam Zheng, 2015/11/20
Re: [Qemu-block] [PATCH for 2.6 0/3] Bitmap clean-up patches for 2.6, Vladimir Sementsov-Ogievskiy, 2015/11/20
- Re: [Qemu-block] [Qemu-devel] [PATCH for 2.6 1/3] backup: Use Bitmap to replace "s->bitmap", (continued)