summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKevin Wolf <kwolf@redhat.com>2017-05-29 14:08:32 +0200
committerMichael Roth <mdroth@linux.vnet.ibm.com>2017-08-03 16:01:28 -0500
commit0ebbef1fa11a19296037695d37053dc18f0d89db (patch)
tree693a5d08d0ef420bef3cec023cde324ef3f16411
parent64945cb5f35f6b52ddc5ef0e4396b93d5bf87e1e (diff)
downloadqemu-0ebbef1fa11a19296037695d37053dc18f0d89db.tar.gz
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> (cherry picked from commit 63c8ef289087a225d445319d047501d4fe593687) Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
-rw-r--r--block/mirror.c7
1 files changed, 6 insertions, 1 deletions
diff --git a/block/mirror.c b/block/mirror.c
index 2173a2f445..4e8f124c30 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;