]>
Commit | Line | Data |
---|---|---|
e74c0f31 | 1 | From 9c4164d74133304aaf7d77001e5ac8a22150df44 Mon Sep 17 00:00:00 2001 |
90a6d957 WB |
2 | From: Kevin Wolf <kwolf@redhat.com> |
3 | Date: Mon, 29 May 2017 14:08:32 +0200 | |
e74c0f31 | 4 | Subject: [PATCH 10/23] mirror: Drop permissions on s->target on completion |
90a6d957 WB |
5 | |
6 | This fixes an assertion failure that was triggered by qemu-iotests 129 | |
7 | on some CI host, while the same test case didn't seem to fail on other | |
8 | hosts. | |
9 | ||
10 | Essentially the problem is that the blk_unref(s->target) in | |
11 | mirror_exit() doesn't necessarily mean that the BlockBackend goes away | |
12 | immediately. It is possible that the job completion was triggered nested | |
13 | in mirror_drain(), which looks like this: | |
14 | ||
15 | BlockBackend *target = s->target; | |
16 | blk_ref(target); | |
17 | blk_drain(target); | |
18 | blk_unref(target); | |
19 | ||
20 | In this case, the write permissions for s->target are retained until | |
21 | after blk_drain(), which makes removing mirror_top_bs fail for the | |
22 | active commit case (can't have a writable backing file in the chain | |
23 | without the filter driver). | |
24 | ||
25 | Explicitly dropping the permissions first means that the additional | |
26 | reference doesn't hurt and the job can complete successfully even if | |
27 | called from the nested blk_drain(). | |
28 | ||
29 | Cc: qemu-stable@nongnu.org | |
30 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | |
31 | Acked-by: Paolo Bonzini <pbonzini@redhat.com> | |
32 | Reviewed-by: Max Reitz <mreitz@redhat.com> | |
33 | --- | |
34 | block/mirror.c | 7 ++++++- | |
35 | 1 file changed, 6 insertions(+), 1 deletion(-) | |
36 | ||
37 | diff --git a/block/mirror.c b/block/mirror.c | |
38 | index 164438f422..779b753b8a 100644 | |
39 | --- a/block/mirror.c | |
40 | +++ b/block/mirror.c | |
41 | @@ -514,7 +514,12 @@ static void mirror_exit(BlockJob *job, void *opaque) | |
42 | ||
43 | /* Remove target parent that still uses BLK_PERM_WRITE/RESIZE before | |
44 | * inserting target_bs at s->to_replace, where we might not be able to get | |
45 | - * these permissions. */ | |
46 | + * these permissions. | |
47 | + * | |
48 | + * Note that blk_unref() alone doesn't necessarily drop permissions because | |
49 | + * we might be running nested inside mirror_drain(), which takes an extra | |
50 | + * reference, so use an explicit blk_set_perm() first. */ | |
51 | + blk_set_perm(s->target, 0, BLK_PERM_ALL, &error_abort); | |
52 | blk_unref(s->target); | |
53 | s->target = NULL; | |
54 | ||
55 | -- | |
56 | 2.11.0 | |
57 |