On Thu, 12/28 14:16, Vladimir Sementsov-Ogievskiy wrote: > 28.12.2017 08:24, Fam Zheng wrote: > > On Wed, 12/20 18:49, Vladimir Sementsov-Ogievskiy wrote: > > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> > > > --- > > > include/block/dirty-bitmap.h | 3 +++ > > > block/dirty-bitmap.c | 25 ++++++++++++++++--------- > > > 2 files changed, 19 insertions(+), 9 deletions(-) > > > > > > diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h > > > index 93d4336505..caf1f3d861 100644 > > > --- a/include/block/dirty-bitmap.h > > > +++ b/include/block/dirty-bitmap.h > > > @@ -92,5 +92,8 @@ bool > > > bdrv_has_changed_persistent_bitmaps(BlockDriverState *bs); > > > BdrvDirtyBitmap *bdrv_dirty_bitmap_next(BlockDriverState *bs, > > > BdrvDirtyBitmap *bitmap); > > > char *bdrv_dirty_bitmap_sha256(const BdrvDirtyBitmap *bitmap, Error > > > **errp); > > > +BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap_locked(BlockDriverState *bs, > > > + BdrvDirtyBitmap > > > *bitmap, > > > + Error **errp); > > > #endif > > > diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c > > > index d83da077d5..fe27ddfb83 100644 > > > --- a/block/dirty-bitmap.c > > > +++ b/block/dirty-bitmap.c > > > @@ -327,15 +327,11 @@ BdrvDirtyBitmap > > > *bdrv_dirty_bitmap_abdicate(BlockDriverState *bs, > > > * The merged parent will be un-frozen, but not explicitly re-enabled. > > > * Called with BQL taken. > > Maybe update the comment as > > > > s/Called with BQL taken/Called within bdrv_dirty_bitmap_lock..unlock/ > > > > and ... > > we have the following comment: > > /* Writing to the list requires the BQL _and_ the dirty_bitmap_mutex. > * Reading from the list can be done with either the BQL or the > * dirty_bitmap_mutex. Modifying a bitmap only requires > * dirty_bitmap_mutex. */ > QemuMutex dirty_bitmap_mutex; > > (I don't understand well the logic, why is it so. Paolo introduced the lock, > but didn't update some functions..) > > so, actually, here we need both BQL and mutex.
Yes, because of that comment my interpretion has been both "BQL and the mutex" whereever we say "within bdrv_dirty_bitmap_lock..unlock", as to some other places in this file. Fam
