ddf19c
From 5774af5a3c713d0c93010c30453812eae6a749cd Mon Sep 17 00:00:00 2001
ddf19c
From: Kevin Wolf <kwolf@redhat.com>
ddf19c
Date: Fri, 13 Mar 2020 12:34:37 +0000
ddf19c
Subject: [PATCH 17/20] block: Fix cross-AioContext blockdev-snapshot
ddf19c
ddf19c
RH-Author: Kevin Wolf <kwolf@redhat.com>
ddf19c
Message-id: <20200313123439.10548-12-kwolf@redhat.com>
ddf19c
Patchwork-id: 94286
ddf19c
O-Subject: [RHEL-AV-8.2.0 qemu-kvm PATCH v2 11/13] block: Fix cross-AioContext blockdev-snapshot
ddf19c
Bugzilla: 1790482 1805143
ddf19c
RH-Acked-by: John Snow <jsnow@redhat.com>
ddf19c
RH-Acked-by: Daniel P. Berrange <berrange@redhat.com>
ddf19c
RH-Acked-by: Peter Krempa <pkrempa@redhat.com>
ddf19c
ddf19c
external_snapshot_prepare() tries to move the overlay to the AioContext
ddf19c
of the backing file (the snapshotted node). However, it's possible that
ddf19c
this doesn't work, but the backing file can instead be moved to the
ddf19c
overlay's AioContext (e.g. opening the backing chain for a mirror
ddf19c
target).
ddf19c
ddf19c
bdrv_append() already indirectly uses bdrv_attach_node(), which takes
ddf19c
care to move nodes to make sure they use the same AioContext and which
ddf19c
tries both directions.
ddf19c
ddf19c
So the problem has a simple fix: Just delete the unnecessary extra
ddf19c
bdrv_try_set_aio_context() call in external_snapshot_prepare() and
ddf19c
instead assert in bdrv_append() that both nodes were indeed moved to the
ddf19c
same AioContext.
ddf19c
ddf19c
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
ddf19c
Message-Id: <20200310113831.27293-6-kwolf@redhat.com>
ddf19c
Tested-by: Peter Krempa <pkrempa@redhat.com>
ddf19c
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
ddf19c
(cherry picked from commit 30dd65f307b647eef8156c4a33bd007823ef85cb)
ddf19c
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
ddf19c
Signed-off-by: Danilo C. L. de Paula <ddepaula@redhat.com>
ddf19c
---
ddf19c
 block.c    |  1 +
ddf19c
 blockdev.c | 16 ----------------
ddf19c
 2 files changed, 1 insertion(+), 16 deletions(-)
ddf19c
ddf19c
diff --git a/block.c b/block.c
ddf19c
index 354d388..ec29b1e 100644
ddf19c
--- a/block.c
ddf19c
+++ b/block.c
ddf19c
@@ -4327,6 +4327,7 @@ void bdrv_replace_node(BlockDriverState *from, BlockDriverState *to,
ddf19c
     bdrv_ref(from);
ddf19c
 
ddf19c
     assert(qemu_get_current_aio_context() == qemu_get_aio_context());
ddf19c
+    assert(bdrv_get_aio_context(from) == bdrv_get_aio_context(to));
ddf19c
     bdrv_drained_begin(from);
ddf19c
 
ddf19c
     /* Put all parents into @list and calculate their cumulative permissions */
ddf19c
diff --git a/blockdev.c b/blockdev.c
ddf19c
index 7918533..c8d4b51 100644
ddf19c
--- a/blockdev.c
ddf19c
+++ b/blockdev.c
ddf19c
@@ -1535,9 +1535,7 @@ static void external_snapshot_prepare(BlkActionState *common,
ddf19c
                              DO_UPCAST(ExternalSnapshotState, common, common);
ddf19c
     TransactionAction *action = common->action;
ddf19c
     AioContext *aio_context;
ddf19c
-    AioContext *old_context;
ddf19c
     uint64_t perm, shared;
ddf19c
-    int ret;
ddf19c
 
ddf19c
     /* 'blockdev-snapshot' and 'blockdev-snapshot-sync' have similar
ddf19c
      * purpose but a different set of parameters */
ddf19c
@@ -1678,20 +1676,6 @@ static void external_snapshot_prepare(BlkActionState *common,
ddf19c
         goto out;
ddf19c
     }
ddf19c
 
ddf19c
-    /* Honor bdrv_try_set_aio_context() context acquisition requirements. */
ddf19c
-    old_context = bdrv_get_aio_context(state->new_bs);
ddf19c
-    aio_context_release(aio_context);
ddf19c
-    aio_context_acquire(old_context);
ddf19c
-
ddf19c
-    ret = bdrv_try_set_aio_context(state->new_bs, aio_context, errp);
ddf19c
-
ddf19c
-    aio_context_release(old_context);
ddf19c
-    aio_context_acquire(aio_context);
ddf19c
-
ddf19c
-    if (ret < 0) {
ddf19c
-        goto out;
ddf19c
-    }
ddf19c
-
ddf19c
     /* This removes our old bs and adds the new bs. This is an operation that
ddf19c
      * can fail, so we need to do it in .prepare; undoing it for abort is
ddf19c
      * always possible. */
ddf19c
-- 
ddf19c
1.8.3.1
ddf19c