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