From dafe096057373d95847e1c99c2fece34be9fc5bb Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 16 Nov 2017 13:00:01 +0100 Subject: [PATCH] block: Fix permissions in image activation Inactive images generally request less permissions for their image files than they would if they were active (in particular, write permissions). Activating the image involves extending the permissions, therefore. drv->bdrv_invalidate_cache() can already require write access to the image file, so we have to update the permissions earlier than that. The current code does it only later, so we have to move up this part. Signed-off-by: Kevin Wolf Reviewed-by: Max Reitz --- block.c | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/block.c b/block.c index 0ed0c27140..752fe6192b 100644 --- a/block.c +++ b/block.c @@ -4174,7 +4174,29 @@ void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp) } } + /* + * Update permissions, they may differ for inactive nodes. + * + * Note that the required permissions of inactive images are always a + * subset of the permissions required after activating the image. This + * allows us to just get the permissions upfront without restricting + * drv->bdrv_invalidate_cache(). + * + * It also means that in error cases, we don't have to try and revert to + * the old permissions (which is an operation that could fail, too). We can + * just keep the extended permissions for the next time that an activation + * of the image is tried. + */ bs->open_flags &= ~BDRV_O_INACTIVE; + bdrv_get_cumulative_perm(bs, &perm, &shared_perm); + ret = bdrv_check_perm(bs, NULL, perm, shared_perm, NULL, &local_err); + if (ret < 0) { + bs->open_flags |= BDRV_O_INACTIVE; + error_propagate(errp, local_err); + return; + } + bdrv_set_perm(bs, perm, shared_perm); + if (bs->drv->bdrv_invalidate_cache) { bs->drv->bdrv_invalidate_cache(bs, &local_err); if (local_err) { @@ -4191,16 +4213,6 @@ void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp) return; } - /* Update permissions, they may differ for inactive nodes */ - bdrv_get_cumulative_perm(bs, &perm, &shared_perm); - ret = bdrv_check_perm(bs, NULL, perm, shared_perm, NULL, &local_err); - if (ret < 0) { - bs->open_flags |= BDRV_O_INACTIVE; - error_propagate(errp, local_err); - return; - } - bdrv_set_perm(bs, perm, shared_perm); - QLIST_FOREACH(parent, &bs->parents, next_parent) { if (parent->role->activate) { parent->role->activate(parent, &local_err); -- 2.30.2