Re: [libvirt] [PATCH v3 07/10] block/dirty-bitmaps: unify qmp_locked and user_locked calls

23.02.2019 3:06, John Snow wrote: > These mean the same thing now. Unify them and rename the merged call > bdrv_dirty_bitmap_busy to indicate semantically what we are describing, > as well as help disambiguate from the various _locked and _unlocked > versions of bitmap helpers that refer to mutex locks. > > Signed-off-by: John Snow <jsnow(a)redhat.com&gt; Hmm, and here, you directly fixed what I've noticed, so my r-b, of course, still applies.

Ha, but I noticed funny thing: > --- > block/dirty-bitmap.c | 40 +++++++++++++++------------------- > blockdev.c | 18 +++++++-------- > include/block/dirty-bitmap.h | 5 ++--- > migration/block-dirty-bitmap.c | 6 ++--- > nbd/server.c | 6 ++--- > 5 files changed, 34 insertions(+), 41 deletions(-) > > diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c > index d92a269753..b3627b0d8c 100644 > --- a/block/dirty-bitmap.c > +++ b/block/dirty-bitmap.c [..] > @@ -265,9 +259,9 @@ int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs, > child->disabled = bitmap->disabled; > bitmap->disabled = true; > > - /* Install the successor and lock the parent */ > + /* Install the successor and mark the parent as busy */ I asked you to fix comment for bdrv_dirty_bitmap_create_successor(), but I didn't noticed this one, I meant the comment to the whole function, placed above it, which now have "Requires that the bitmap is not user_locked and has no successor." So, it should be updated too. and with it: Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov(a)virtuozzo.com&gt;

> bitmap->successor = child; > - bitmap->qmp_locked = true; > + bitmap->busy = true; > return 0; > } >