[v2,16/33] block: Pull out bdrv_default_perms_for_storage()

Commit Message

Max Reitz Feb. 4, 2020, 5:08 p.m. UTC

Signed-off-by: Max Reitz <[e-mail]>
---
 block.c | 71 +++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 46 insertions(+), 25 deletions(-)

Comments

Eric Blake Feb. 5, 2020, 9:02 p.m. UTC | #1

On 2/4/20 11:08 AM, Max Reitz wrote:
> Signed-off-by: Max Reitz <[e-mail]>
> ---

Commit message is short.

>   block.c | 71 +++++++++++++++++++++++++++++++++++++--------------------
>   1 file changed, 46 insertions(+), 25 deletions(-)
> 

Reviewed-by: Eric Blake <[e-mail]>

Max Reitz Feb. 6, 2020, 11:20 a.m. UTC | #2

On 05.02.20 22:02, Eric Blake wrote:
> On 2/4/20 11:08 AM, Max Reitz wrote:
>> Signed-off-by: Max Reitz <[e-mail]>
>> ---
> 
> Commit message is short.

How about:

Right now, bdrv_format_default_perms() is used by format parents
(generally). We want to switch to a model where most parents use a
single BdrvChildClass, which then decides the permissions based on the
child role. To do so, we have to split bdrv_format_default_perms() into
separate functions for each such role.

Note that bdrv_default_perms_for_storage() currently handles all DATA |
METADATA children.  A follow-up patch is going to split it further into
one function for each case.

Max

Patch

diff --git a/block.c b/block.c
index b26ec10cce..b7999696d4 100644
--- a/block.c
+++ b/block.c
@@ -2318,6 +2318,50 @@  static void bdrv_default_perms_for_backing(BlockDriverState *bs, BdrvChild *c,
     *nshared = shared;
 }
 
+static void bdrv_default_perms_for_storage(BlockDriverState *bs, BdrvChild *c,
+                                           const BdrvChildClass *child_class,
+                                           BdrvChildRole role,
+                                           BlockReopenQueue *reopen_queue,
+                                           uint64_t perm, uint64_t shared,
+                                           uint64_t *nperm, uint64_t *nshared)
+{
+    int flags;
+
+    assert(child_class == &child_file ||
+           (child_class == &child_of_bds &&
+            (role & (BDRV_CHILD_METADATA | BDRV_CHILD_DATA))));
+
+    flags = bdrv_reopen_get_flags(reopen_queue, bs);
+
+    /*
+     * Apart from the modifications below, the same permissions are
+     * forwarded and left alone as for filters
+     */
+    bdrv_filter_default_perms(bs, c, child_class, role, reopen_queue,
+                              perm, shared, &perm, &shared);
+
+    /* Format drivers may touch metadata even if the guest doesn't write */
+    if (bdrv_is_writable_after_reopen(bs, reopen_queue)) {
+        perm |= BLK_PERM_WRITE | BLK_PERM_RESIZE;
+    }
+
+    /*
+     * bs->file always needs to be consistent because of the metadata. We
+     * can never allow other users to resize or write to it.
+     */
+    if (!(flags & BDRV_O_NO_IO)) {
+        perm |= BLK_PERM_CONSISTENT_READ;
+    }
+    shared &= ~(BLK_PERM_WRITE | BLK_PERM_RESIZE);
+
+    if (bs->open_flags & BDRV_O_INACTIVE) {
+        shared |= BLK_PERM_WRITE | BLK_PERM_RESIZE;
+    }
+
+    *nperm = perm;
+    *nshared = shared;
+}
+
 void bdrv_format_default_perms(BlockDriverState *bs, BdrvChild *c,
                                const BdrvChildClass *child_class,
                                BdrvChildRole role,
@@ -2329,31 +2373,8 @@  void bdrv_format_default_perms(BlockDriverState *bs, BdrvChild *c,
     assert(child_class == &child_backing || child_class == &child_file);
 
     if (!backing) {
-        int flags = bdrv_reopen_get_flags(reopen_queue, bs);
-
-        /* Apart from the modifications below, the same permissions are
-         * forwarded and left alone as for filters */
-        bdrv_filter_default_perms(bs, c, child_class, role, reopen_queue,
-                                  perm, shared, &perm, &shared);
-
-        /* Format drivers may touch metadata even if the guest doesn't write */
-        if (bdrv_is_writable_after_reopen(bs, reopen_queue)) {
-            perm |= BLK_PERM_WRITE | BLK_PERM_RESIZE;
-        }
-
-        /* bs->file always needs to be consistent because of the metadata. We
-         * can never allow other users to resize or write to it. */
-        if (!(flags & BDRV_O_NO_IO)) {
-            perm |= BLK_PERM_CONSISTENT_READ;
-        }
-        shared &= ~(BLK_PERM_WRITE | BLK_PERM_RESIZE);
-
-        if (bs->open_flags & BDRV_O_INACTIVE) {
-            shared |= BLK_PERM_WRITE | BLK_PERM_RESIZE;
-        }
-
-        *nperm = perm;
-        *nshared = shared;
+        bdrv_default_perms_for_storage(bs, c, child_class, role, reopen_queue,
+                                       perm, shared, nperm, nshared);
     } else {
         bdrv_default_perms_for_backing(bs, c, child_class, role, reopen_queue,
                                        perm, shared, nperm, nshared);