diff options
author | Kevin Wolf <kwolf@redhat.com> | 2019-12-17 15:06:38 +0100 |
---|---|---|
committer | Kevin Wolf <kwolf@redhat.com> | 2019-12-19 18:04:25 +0100 |
commit | 7bb4941ace471fc7dd6ded4749b95b9622baa6ed (patch) | |
tree | 514dfbdd63c9eadd442a2fc2f04ec77cad1e80c4 | |
parent | 7fe6bb7a34599714afa8d4e5fac612ec1fdbdb5c (diff) | |
download | qemu-7bb4941ace471fc7dd6ded4749b95b9622baa6ed.zip qemu-7bb4941ace471fc7dd6ded4749b95b9622baa6ed.tar.gz qemu-7bb4941ace471fc7dd6ded4749b95b9622baa6ed.tar.bz2 |
block: Activate recursively even for already active nodes
bdrv_invalidate_cache_all() assumes that all nodes in a given subtree
are either active or inactive when it starts. Therefore, as soon as it
arrives at an already active node, it stops.
However, this assumption is wrong. For example, it's possible to take a
snapshot of an inactive node, which results in an active overlay over an
inactive backing file. The active overlay is probably also the root node
of an inactive BlockBackend (blk->disable_perm == true).
In this case, bdrv_invalidate_cache_all() does not need to do anything
to activate the overlay node, but it still needs to recurse into the
children and the parents to make sure that after returning success,
really everything is activated.
Cc: qemu-stable@nongnu.org
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
-rw-r--r-- | block.c | 50 |
1 files changed, 24 insertions, 26 deletions
@@ -5335,10 +5335,6 @@ static void coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs, return; } - if (!(bs->open_flags & BDRV_O_INACTIVE)) { - return; - } - QLIST_FOREACH(child, &bs->children, next) { bdrv_co_invalidate_cache(child->bs, &local_err); if (local_err) { @@ -5360,34 +5356,36 @@ static void coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs, * 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, 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_co_invalidate_cache) { - bs->drv->bdrv_co_invalidate_cache(bs, &local_err); - if (local_err) { + if (bs->open_flags & BDRV_O_INACTIVE) { + bs->open_flags &= ~BDRV_O_INACTIVE; + bdrv_get_cumulative_perm(bs, &perm, &shared_perm); + ret = bdrv_check_perm(bs, NULL, perm, shared_perm, NULL, 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); - FOR_EACH_DIRTY_BITMAP(bs, bm) { - bdrv_dirty_bitmap_skip_store(bm, false); - } + if (bs->drv->bdrv_co_invalidate_cache) { + bs->drv->bdrv_co_invalidate_cache(bs, &local_err); + if (local_err) { + bs->open_flags |= BDRV_O_INACTIVE; + error_propagate(errp, local_err); + return; + } + } - ret = refresh_total_sectors(bs, bs->total_sectors); - if (ret < 0) { - bs->open_flags |= BDRV_O_INACTIVE; - error_setg_errno(errp, -ret, "Could not refresh total sector count"); - return; + FOR_EACH_DIRTY_BITMAP(bs, bm) { + bdrv_dirty_bitmap_skip_store(bm, false); + } + + ret = refresh_total_sectors(bs, bs->total_sectors); + if (ret < 0) { + bs->open_flags |= BDRV_O_INACTIVE; + error_setg_errno(errp, -ret, "Could not refresh total sector count"); + return; + } } QLIST_FOREACH(parent, &bs->parents, next_parent) { |