aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKevin Wolf <kwolf@redhat.com>2019-01-31 15:16:10 +0100
committerKevin Wolf <kwolf@redhat.com>2019-02-01 13:46:45 +0100
commit78fc3b3a26c145eebcdee992988644974b243a74 (patch)
tree0100f643023837f66fb19488ea581f09e3a5e9ee
parent039be85c410bfb4b53cdee2083b4245e0d4e4181 (diff)
block: Fix invalidate_cache error path for parent activation
bdrv_co_invalidate_cache() clears the BDRV_O_INACTIVE flag before actually activating a node so that the correct permissions etc. are taken. In case of errors, the flag must be restored so that the next call to bdrv_co_invalidate_cache() retries activation. Restoring the flag was missing in the error path for a failed parent->role->activate() call. The consequence is that this attempt to activate all images correctly fails because we still set errp, however on the next attempt BDRV_O_INACTIVE is already clear, so we return success without actually retrying the failed action. An example where this is observable in practice is migration to a QEMU instance that has a raw format block node attached to a guest device with share-rw=off (the default) while another process holds BLK_PERM_WRITE for the same image. In this case, all activation steps before parent->role->activate() succeed because raw can tolerate other writers to the image. Only the parent callback (in particular blk_root_activate()) tries to implement the share-rw=on property and requests exclusive write permissions. This fails when the migration completes and correctly displays an error. However, a manual 'cont' will incorrectly resume the VM without calling blk_root_activate() again. This case is described in more detail in the following bug report: https://bugzilla.redhat.com/show_bug.cgi?id=1531888 Fix this by correctly restoring the BDRV_O_INACTIVE flag in the error path. Cc: qemu-stable@nongnu.org Signed-off-by: Kevin Wolf <kwolf@redhat.com> Tested-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
-rw-r--r--block.c1
1 files changed, 1 insertions, 0 deletions
diff --git a/block.c b/block.c
index 5a4d27173e..b67d9b7b65 100644
--- a/block.c
+++ b/block.c
@@ -4697,6 +4697,7 @@ static void coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs,
if (parent->role->activate) {
parent->role->activate(parent, &local_err);
if (local_err) {
+ bs->open_flags |= BDRV_O_INACTIVE;
error_propagate(errp, local_err);
return;
}