Blame SOURCES/kvm-block-Make-permission-changes-in-reopen-less-wrong.patch

7711c0
From f64042620963059859d0bbae60e8655ccc44736e Mon Sep 17 00:00:00 2001
7711c0
From: Kevin Wolf <kwolf@redhat.com>
7711c0
Date: Fri, 15 Mar 2019 18:10:01 +0100
7711c0
Subject: [PATCH 005/163] block: Make permission changes in reopen less wrong
7711c0
7711c0
RH-Author: Kevin Wolf <kwolf@redhat.com>
7711c0
Message-id: <20190315181010.14964-6-kwolf@redhat.com>
7711c0
Patchwork-id: 84882
7711c0
O-Subject: [RHEL-7.7 qemu-kvm-rhev PATCH 05/14] block: Make permission changes in reopen less wrong
7711c0
Bugzilla: 1685989
7711c0
RH-Acked-by: John Snow <jsnow@redhat.com>
7711c0
RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
7711c0
RH-Acked-by: Miroslav Rezanina <mrezanin@redhat.com>
7711c0
7711c0
The way that reopen interacts with permission changes has one big
7711c0
problem: Both operations are recursive, and the permissions are changes
7711c0
for each node in the reopen queue.
7711c0
7711c0
For a simple graph that consists just of parent and child,
7711c0
.bdrv_check_perm will be called twice for the child, once recursively
7711c0
when adjusting the permissions of parent, and once again when the child
7711c0
itself is reopened.
7711c0
7711c0
Even worse, the first .bdrv_check_perm call happens before
7711c0
.bdrv_reopen_prepare was called for the child and the second one is
7711c0
called afterwards.
7711c0
7711c0
Making sure that .bdrv_check_perm (and the other permission callbacks)
7711c0
are called only once is hard. We can cope with multiple calls right now,
7711c0
but as soon as file-posix gets a dynamic auto-read-only that may need to
7711c0
open a new file descriptor, we get the additional requirement that all
7711c0
of them are after the .bdrv_reopen_prepare call.
7711c0
7711c0
So reorder things in bdrv_reopen_multiple() to first call
7711c0
.bdrv_reopen_prepare for all involved nodes and only then adjust
7711c0
permissions.
7711c0
7711c0
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
7711c0
(cherry picked from commit 69b736e76567ecbc9b9e55570bc0afc840614a98)
7711c0
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
7711c0
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
7711c0
---
7711c0
 block.c | 35 ++++++++++++++++++++++++-----------
7711c0
 1 file changed, 24 insertions(+), 11 deletions(-)
7711c0
7711c0
diff --git a/block.c b/block.c
7711c0
index 2f1b4d1..b31124c 100644
7711c0
--- a/block.c
7711c0
+++ b/block.c
7711c0
@@ -1661,6 +1661,7 @@ static void bdrv_child_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared);
7711c0
 
7711c0
 typedef struct BlockReopenQueueEntry {
7711c0
      bool prepared;
7711c0
+     bool perms_checked;
7711c0
      BDRVReopenState state;
7711c0
      QSIMPLEQ_ENTRY(BlockReopenQueueEntry) entry;
7711c0
 } BlockReopenQueueEntry;
7711c0
@@ -3072,6 +3073,16 @@ int bdrv_reopen_multiple(AioContext *ctx, BlockReopenQueue *bs_queue, Error **er
7711c0
         bs_entry->prepared = true;
7711c0
     }
7711c0
 
7711c0
+    QSIMPLEQ_FOREACH(bs_entry, bs_queue, entry) {
7711c0
+        BDRVReopenState *state = &bs_entry->state;
7711c0
+        ret = bdrv_check_perm(state->bs, bs_queue, state->perm,
7711c0
+                              state->shared_perm, NULL, errp);
7711c0
+        if (ret < 0) {
7711c0
+            goto cleanup_perm;
7711c0
+        }
7711c0
+        bs_entry->perms_checked = true;
7711c0
+    }
7711c0
+
7711c0
     /* If we reach this point, we have success and just need to apply the
7711c0
      * changes
7711c0
      */
7711c0
@@ -3080,7 +3091,20 @@ int bdrv_reopen_multiple(AioContext *ctx, BlockReopenQueue *bs_queue, Error **er
7711c0
     }
7711c0
 
7711c0
     ret = 0;
7711c0
+cleanup_perm:
7711c0
+    QSIMPLEQ_FOREACH_SAFE(bs_entry, bs_queue, entry, next) {
7711c0
+        BDRVReopenState *state = &bs_entry->state;
7711c0
+
7711c0
+        if (!bs_entry->perms_checked) {
7711c0
+            continue;
7711c0
+        }
7711c0
 
7711c0
+        if (ret == 0) {
7711c0
+            bdrv_set_perm(state->bs, state->perm, state->shared_perm);
7711c0
+        } else {
7711c0
+            bdrv_abort_perm_update(state->bs);
7711c0
+        }
7711c0
+    }
7711c0
 cleanup:
7711c0
     QSIMPLEQ_FOREACH_SAFE(bs_entry, bs_queue, entry, next) {
7711c0
         if (ret) {
7711c0
@@ -3297,12 +3321,6 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue,
7711c0
         } while ((entry = qdict_next(reopen_state->options, entry)));
7711c0
     }
7711c0
 
7711c0
-    ret = bdrv_check_perm(reopen_state->bs, queue, reopen_state->perm,
7711c0
-                          reopen_state->shared_perm, NULL, errp);
7711c0
-    if (ret < 0) {
7711c0
-        goto error;
7711c0
-    }
7711c0
-
7711c0
     ret = 0;
7711c0
 
7711c0
 error:
7711c0
@@ -3352,9 +3370,6 @@ void bdrv_reopen_commit(BDRVReopenState *reopen_state)
7711c0
 
7711c0
     bdrv_refresh_limits(bs, NULL);
7711c0
 
7711c0
-    bdrv_set_perm(reopen_state->bs, reopen_state->perm,
7711c0
-                  reopen_state->shared_perm);
7711c0
-
7711c0
     new_can_write =
7711c0
         !bdrv_is_read_only(bs) && !(bdrv_get_flags(bs) & BDRV_O_INACTIVE);
7711c0
     if (!old_can_write && new_can_write && drv->bdrv_reopen_bitmaps_rw) {
7711c0
@@ -3386,8 +3401,6 @@ void bdrv_reopen_abort(BDRVReopenState *reopen_state)
7711c0
     if (drv->bdrv_reopen_abort) {
7711c0
         drv->bdrv_reopen_abort(reopen_state);
7711c0
     }
7711c0
-
7711c0
-    bdrv_abort_perm_update(reopen_state->bs);
7711c0
 }
7711c0
 
7711c0
 
7711c0
-- 
7711c0
1.8.3.1
7711c0