58 lines
2.1 KiB
Diff
58 lines
2.1 KiB
Diff
From 9c4164d74133304aaf7d77001e5ac8a22150df44 Mon Sep 17 00:00:00 2001
|
|
From: Kevin Wolf <kwolf@redhat.com>
|
|
Date: Mon, 29 May 2017 14:08:32 +0200
|
|
Subject: [PATCH 10/23] mirror: Drop permissions on s->target on completion
|
|
|
|
This fixes an assertion failure that was triggered by qemu-iotests 129
|
|
on some CI host, while the same test case didn't seem to fail on other
|
|
hosts.
|
|
|
|
Essentially the problem is that the blk_unref(s->target) in
|
|
mirror_exit() doesn't necessarily mean that the BlockBackend goes away
|
|
immediately. It is possible that the job completion was triggered nested
|
|
in mirror_drain(), which looks like this:
|
|
|
|
BlockBackend *target = s->target;
|
|
blk_ref(target);
|
|
blk_drain(target);
|
|
blk_unref(target);
|
|
|
|
In this case, the write permissions for s->target are retained until
|
|
after blk_drain(), which makes removing mirror_top_bs fail for the
|
|
active commit case (can't have a writable backing file in the chain
|
|
without the filter driver).
|
|
|
|
Explicitly dropping the permissions first means that the additional
|
|
reference doesn't hurt and the job can complete successfully even if
|
|
called from the nested blk_drain().
|
|
|
|
Cc: qemu-stable@nongnu.org
|
|
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
|
|
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
|
|
Reviewed-by: Max Reitz <mreitz@redhat.com>
|
|
---
|
|
block/mirror.c | 7 ++++++-
|
|
1 file changed, 6 insertions(+), 1 deletion(-)
|
|
|
|
diff --git a/block/mirror.c b/block/mirror.c
|
|
index 164438f422..779b753b8a 100644
|
|
--- a/block/mirror.c
|
|
+++ b/block/mirror.c
|
|
@@ -514,7 +514,12 @@ static void mirror_exit(BlockJob *job, void *opaque)
|
|
|
|
/* Remove target parent that still uses BLK_PERM_WRITE/RESIZE before
|
|
* inserting target_bs at s->to_replace, where we might not be able to get
|
|
- * these permissions. */
|
|
+ * these permissions.
|
|
+ *
|
|
+ * Note that blk_unref() alone doesn't necessarily drop permissions because
|
|
+ * we might be running nested inside mirror_drain(), which takes an extra
|
|
+ * reference, so use an explicit blk_set_perm() first. */
|
|
+ blk_set_perm(s->target, 0, BLK_PERM_ALL, &error_abort);
|
|
blk_unref(s->target);
|
|
s->target = NULL;
|
|
|
|
--
|
|
2.11.0
|
|
|