From e6ae3c4666f35c30ac171e9994b5116d9594c8ce Mon Sep 17 00:00:00 2001
From: Kevin Wolf <kwolf@redhat.com>
Date: Wed, 31 May 2017 10:55:21 +0200
Subject: [PATCH 03/13] mirror: Drop permissions on s->target on completion
RH-Author: Kevin Wolf <kwolf@redhat.com>
Message-id: <1496228121-28770-2-git-send-email-kwolf@redhat.com>
Patchwork-id: 75443
O-Subject: [RHEL-7.4 qemu-kvm-rhev PATCH] mirror: Drop permissions on s->target on completion
Bugzilla: 1456456
RH-Acked-by: Jeffrey Cody <jcody@redhat.com>
RH-Acked-by: Fam Zheng <famz@redhat.com>
RH-Acked-by: Max Reitz <mreitz@redhat.com>
RH-Acked-by: John Snow <jsnow@redhat.com>
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: Miroslav Rezanina <mrezanin@redhat.com>
---
block/mirror.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/block/mirror.c b/block/mirror.c
index 2173a2f..4e8f124 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;
--
1.8.3.1