From 7d5b5261104235cd6cc86930ab0d0a7372bfe04b Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Wed, 23 Aug 2017 21:42:42 +0800 Subject: [PATCH] block: Update open_flags after ->inactivate() callback In the ->inactivate() callbacks, permissions are updated, which typically involves a recursive check of the whole graph. Setting BDRV_O_INACTIVE right before doing that creates a state that bdrv_is_writable() returns false, which causes permission update failure. Reorder them so the flag is updated after calling the function. Note that this doesn't break the assert in bdrv_child_cb_inactivate() because for any specific BDS, we still update its flags first before calling ->inactivate() on it one level deeper in the recursion. Signed-off-by: Stefan Hajnoczi Signed-off-by: Fam Zheng Message-Id: <20170823134242.12080-5-famz@redhat.com> Tested-by: Dr. David Alan Gilbert Signed-off-by: Eric Blake --- block.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/block.c b/block.c index 3615a6809e..3308814bba 100644 --- a/block.c +++ b/block.c @@ -4085,21 +4085,20 @@ static int bdrv_inactivate_recurse(BlockDriverState *bs, } } - if (setting_flag) { + if (setting_flag && !(bs->open_flags & BDRV_O_INACTIVE)) { uint64_t perm, shared_perm; - bs->open_flags |= BDRV_O_INACTIVE; - QLIST_FOREACH(parent, &bs->parents, next_parent) { if (parent->role->inactivate) { ret = parent->role->inactivate(parent); if (ret < 0) { - bs->open_flags &= ~BDRV_O_INACTIVE; return ret; } } } + bs->open_flags |= BDRV_O_INACTIVE; + /* Update permissions, they may differ for inactive nodes */ bdrv_get_cumulative_perm(bs, &perm, &shared_perm); bdrv_check_perm(bs, perm, shared_perm, NULL, &error_abort);