Re: [Qemu-devel] [Qemu-block] [PATCH v3 09/15] block: Support meta dirty bitmap

[Fam Zheng]

On Mon, 02/29 18:00, John Snow wrote:
> 
> 
> On 02/27/2016 04:20 AM, Fam Zheng wrote:
> > The added group of operations enables tracking of the changed bits in
> > the dirty bitmap.
> > 
> > Signed-off-by: Fam Zheng <address@hidden>
> > ---
> >  block/dirty-bitmap.c         | 51 
> > ++++++++++++++++++++++++++++++++++++++++++++
> >  include/block/dirty-bitmap.h |  9 ++++++++
> >  2 files changed, 60 insertions(+)
> > 
> > diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> > index 16f73b2..5f19320 100644
> > --- a/block/dirty-bitmap.c
> > +++ b/block/dirty-bitmap.c
> > @@ -38,6 +38,7 @@
> >   */
> >  struct BdrvDirtyBitmap {
> >      HBitmap *bitmap;            /* Dirty sector bitmap implementation */
> > +    HBitmap *meta;              /* Meta dirty bitmap */
> >      BdrvDirtyBitmap *successor; /* Anonymous child; implies frozen status 
> > */
> >      char *name;                 /* Optional non-empty unique ID */
> >      int64_t size;               /* Size of the bitmap (Number of sectors) 
> > */
> > @@ -103,6 +104,56 @@ BdrvDirtyBitmap 
> > *bdrv_create_dirty_bitmap(BlockDriverState *bs,
> >      return bitmap;
> >  }
> >  
> > +/* bdrv_create_meta_dirty_bitmap
> > + *
> > + * Create a meta dirty bitmap that tracks the changes of bits in @bitmap. 
> > I.e.
> > + * when a dirty status bit in @bitmap is changed (either from reset to set 
> > or
> > + * the other way around), its respective meta dirty bitmap bit will be 
> > marked
> > + * dirty as well.
> > + *
> > + * @bitmap: the block dirty bitmap for which to create a meta dirty bitmap.
> > + * @chunk_size: how many bytes of bitmap data does each bit in the meta 
> > bitmap
> > + * track.
> > + */
> > +void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap,
> > +                                   int chunk_size)
> > +{
> > +    assert(!bitmap->meta);
> > +    bitmap->meta = hbitmap_create_meta(bitmap->bitmap,
> > +                                       chunk_size * BITS_PER_BYTE);
> > +}
> > +
> > +void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap)
> > +{
> > +    assert(bitmap->meta);
> > +    hbitmap_free_meta(bitmap->bitmap);
> > +    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;
> > +}
> > +
> > +void bdrv_dirty_bitmap_reset_meta(BlockDriverState *bs,
> > +                                  BdrvDirtyBitmap *bitmap, int64_t sector,
> > +                                  int nb_sectors)
> > +{
> > +    hbitmap_reset(bitmap->meta, sector, nb_sectors);
> > +}
> > +
> >  bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap)
> >  {
> >      return bitmap->successor;
> > diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
> > index e1dbd8e..3b27742 100644
> > --- a/include/block/dirty-bitmap.h
> > +++ b/include/block/dirty-bitmap.h
> > @@ -9,6 +9,9 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState 
> > *bs,
> >                                            uint32_t granularity,
> >                                            const char *name,
> >                                            Error **errp);
> > +void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap,
> > +                                   int chunk_size);
> > +void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap);
> >  int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,
> >                                         BdrvDirtyBitmap *bitmap,
> >                                         Error **errp);
> > @@ -37,6 +40,12 @@ void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,
> >                             int64_t cur_sector, int nr_sectors);
> >  void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap,
> >                               int64_t cur_sector, int nr_sectors);
> > +int bdrv_dirty_bitmap_get_meta(BlockDriverState *bs,
> > +                               BdrvDirtyBitmap *bitmap, int64_t sector,
> > +                               int nb_sectors);
> > +void bdrv_dirty_bitmap_reset_meta(BlockDriverState *bs,
> > +                                  BdrvDirtyBitmap *bitmap, int64_t sector,
> > +                                  int nb_sectors);
> >  BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap *bitmap,
> >                                           uint64_t first_sector);
> >  void bdrv_dirty_iter_free(BdrvDirtyBitmapIter *iter);
> > 
> 
> Do we need to amend the teardown in bdrv_do_release_matching_dirty_bitmap?

Will add an "assert(!bm->meta)" there.

Fam