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