On 01/20/2016 01:07 AM, Fam Zheng wrote: > On Thu, 01/07 14:30, John Snow wrote: >>> +void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap) >>> +{ >>> + assert(bitmap->meta); >>> + hbitmap_free(bitmap->meta); >> >> This leaves a dangling pointer inside the Hbitmap, no? > > Yes, will fix. > >> >>> + bitmap->meta = NULL; >>> +} >>> + >>> +int bdrv_dirty_bitmap_get_meta(BlockDriverState *bs, >>> + BdrvDirtyBitmap *bitmap, int64_t sector, >>> + int nb_sectors) >>> +{ >>> + uint64_t i; >>> + int gran = bdrv_dirty_bitmap_granularity(bitmap) >> BDRV_SECTOR_BITS; >>> + >>> + /* To optimize: we can make hbitmap to internally check the range in a >>> + * coarse level, or at least do it word by word. */ >>> + for (i = sector; i < sector + nb_sectors; i += gran) { >>> + if (hbitmap_get(bitmap->meta, i)) { >>> + return true; >>> + } >>> + } >>> + return false; >>> +} >>> + >> >> In essence get_meta() is a greedy algorithm that simply returns true if >> anything is set between [sector, sector + nb_sectors], yes? >> >> Is this more useful than just using an iterator directly on the >> meta-bitmap? >> >> I haven't finished reading but, I imagine that: >> >> - If we need to check to see what is dirty specifically, we can just use >> the iterator. If the iterator doesn't return anything, we know it's >> empty. If it does return, we know exactly what's dirty. >> - If we need to explicitly check for emptiness in general, we can use >> the internal popcount. >> >> >> I'm not sure when a 'dirty range bool' will be explicitly useful all by >> itself, but maybe that becomes obvious later. > > It's for the meta bitmap user to decide. In the case of persistent dirty > bitmap > driver, I simply check whether the range of write request is meta-dirty, and > write the corresponding dirty bitmap range accordingly, rather than splitting > one write req into potentially multiple bit ranges that are meta-dirty. I > think > this is reasonable, hence the interface. >
OK, I think I see what the use case is, thanks.