14.02.2019 2:23, John Snow wrote:
Instead of implying a locked status, make it explicit.
locked interferes with bitmap mutex, so may be better "qmp_locked state", but not sure.
Now, bitmaps in use by migration, NBD or backup operations are all treated the same way with the same code paths.
Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Hmm. Isn't it better to make successor-related staff unrelated to locking at all? So, backup will call set_qmp_locked like others? And then do create_successor, abdicate, reclaim, whatever it wants, and finally set_qmp_locked(false) ? To make it work even more in the same path. But it may be done separately, if we want.
--- block/dirty-bitmap.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c index bb2e19e0d8..2042c62602 100644 --- a/block/dirty-bitmap.c +++ b/block/dirty-bitmap.c @@ -188,10 +188,8 @@ bool bdrv_dirty_bitmap_has_successor(BdrvDirtyBitmap *bitmap) return bitmap->successor; }
-/* Both conditions disallow user-modification via QMP. */ bool bdrv_dirty_bitmap_user_locked(BdrvDirtyBitmap *bitmap) { - return bdrv_dirty_bitmap_has_successor(bitmap) || - bdrv_dirty_bitmap_qmp_locked(bitmap); + return bdrv_dirty_bitmap_qmp_locked(bitmap); }
void bdrv_dirty_bitmap_set_qmp_locked(BdrvDirtyBitmap *bitmap, bool qmp_locked) @@ -266,8 +264,9 @@ int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs, child->disabled = bitmap->disabled; bitmap->disabled = true;
- /* Install the successor and freeze the parent */ + /* Install the successor and lock the parent */ bitmap->successor = child; + bitmap->qmp_locked = true; return 0; }
@@ -321,6 +320,7 @@ BdrvDirtyBitmap *bdrv_dirty_bitmap_abdicate(BlockDriverState *bs, bitmap->successor = NULL; successor->persistent = bitmap->persistent; bitmap->persistent = false; + bitmap->qmp_locked = false; bdrv_release_dirty_bitmap(bs, bitmap);
return successor; @@ -349,6 +349,7 @@ BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap_locked(BlockDriverState *bs, }
parent->disabled = successor->disabled; + parent->qmp_locked = false; bdrv_release_dirty_bitmap_locked(successor); parent->successor = NULL;
-- Best regards, Vladimir