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.
