On Tue, 11/24 12:12, Vladimir Sementsov-Ogievskiy wrote: > On 24.11.2015 05:28, 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 > >it > >more like a HGranularityBitmap rather than HBitmap, and is unnecessarily > >complex to work on. > > > >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 > >the > >dirty bitmap interface, for example to serialize and deserialize for > >persistence. > > > >Fam > > But what is the relation from this to adding iterator interface? > This seems like this patch do two different things: > 1) granularity changes, described in quotation above > 2) adding iterator related things, described in commit message >
OK, let's try to split this so we still have the BdrvDirtyBitmapIter change even if the granularity movement is rejected. Fam