From 7bcc4bb06fb8455874578fe3ad722600081e8d1c Mon Sep 17 00:00:00 2001

From: John Snow <jsnow@redhat.com>

Date: Wed, 3 Apr 2019 18:18:42 +0200

Subject: [PATCH 137/163] block/dirty-bitmap: change semantics of enabled

 predicate

RH-Author: John Snow <jsnow@redhat.com>

Message-id: <20190403181857.9693-7-jsnow@redhat.com>

Patchwork-id: 85427

O-Subject: [RHEL-7.7 qemu-kvm-rhev PATCH 06/21] block/dirty-bitmap: change semantics of enabled predicate

Bugzilla: 1677073

RH-Acked-by: Max Reitz <mreitz@redhat.com>

RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>

RH-Acked-by: Sergio Lopez Pascual <slp@redhat.com>

Currently, the enabled predicate means something like:

"the QAPI status of the bitmap is ACTIVE."

After this patch, it should mean exclusively:

"This bitmap is recording guest writes, and is allowed to do so."

In many places, this is how this predicate was already used.

Internal usages of the bitmap QPI can call user_locked to find out if

the bitmap is in use by an operation.

To accommodate this, modify the create_successor routine to now

explicitly disable the parent bitmap at creation time.

Justifications:

1. bdrv_dirty_bitmap_status suffers no change from the lack of

   1:1 parity with the new predicates because of the order in which

   the predicates are checked. This is now only for compatibility.

2. bdrv_set_dirty() is unchanged: pre-patch, it was skipping bitmaps that were

   disabled or had a successor, while post-patch it is only skipping bitmaps

   that are disabled. To accommodate this, create_successor now ensures that

   any bitmap with a successor is explicitly disabled.

3. qcow2_store_persistent_dirty_bitmaps: No functional change. This function

   cares only about the literal enabled bit, and makes no effort to check if

   the bitmap is in-use or not. After this patch there are still no ways to

   produce an enabled bitmap with a successor.

4. block_dirty_bitmap_enable_prepare

   block_dirty_bitmap_disable_prepare

   init_dirty_bitmap_migration

   nbd_export_new

   These functions care about the literal enabled bit,

   and already check user_locked separately.

Signed-off-by: John Snow <jsnow@redhat.com>

Reviewed-by: Eric Blake <eblake@redhat.com>

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Message-id: 20190223000614.13894-5-jsnow@redhat.com

Signed-off-by: John Snow <jsnow@redhat.com>

(cherry picked from commit 8b2e20f64f25a5bf9a7cd45b4babdf2d7416f7ad)

Signed-off-by: John Snow <jsnow@redhat.com>

Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>

---

 block/dirty-bitmap.c | 9 +++++++--

 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c

index 7dc5b55..fa411f9 100644

--- a/block/dirty-bitmap.c

+++ b/block/dirty-bitmap.c

@@ -209,7 +209,7 @@ bool bdrv_dirty_bitmap_qmp_locked(BdrvDirtyBitmap *bitmap)

 /* Called with BQL taken.  */

 bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap)

 {

-    return !(bitmap->disabled || bitmap->successor);

+    return !bitmap->disabled;

 }

 /* Called with BQL taken.  */

@@ -236,6 +236,7 @@ static bool bdrv_dirty_bitmap_recording(BdrvDirtyBitmap *bitmap)

 /**

  * Create a successor bitmap destined to replace this bitmap after an operation.

  * Requires that the bitmap is not user_locked and has no successor.

+ * The successor will be enabled if the parent bitmap was.

  * Called with BQL taken.

  */

 int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,

@@ -264,6 +265,7 @@ int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,

     /* Successor will be on or off based on our current state. */

     child->disabled = bitmap->disabled;

+    bitmap->disabled = true;

     /* Install the successor and freeze the parent */

     bitmap->successor = child;

@@ -329,7 +331,8 @@ BdrvDirtyBitmap *bdrv_dirty_bitmap_abdicate(BlockDriverState *bs,

 /**

  * In cases of failure where we can no longer safely delete the parent,

  * we may wish to re-join the parent and child/successor.

- * The merged parent will not be user_locked, nor explicitly re-enabled.

+ * The merged parent will not be user_locked.

+ * The marged parent will be enabled if and only if the successor was enabled.

  * Called within bdrv_dirty_bitmap_lock..unlock and with BQL taken.

  */

 BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap_locked(BlockDriverState *bs,

@@ -347,6 +350,8 @@ BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap_locked(BlockDriverState *bs,

         error_setg(errp, "Merging of parent and successor bitmap failed");

         return NULL;

     }

+

+    parent->disabled = successor->disabled;

     bdrv_release_dirty_bitmap_locked(successor);

     parent->successor = NULL;

-- 

1.8.3.1