diff mbox series

[v2,15/33] block: Pull out bdrv_default_perms_for_backing()

Message ID 20200204170848.614480-16-mreitz@redhat.com (mailing list archive)
State New, archived
Headers show
Series block: Introduce real BdrvChildRole | expand

Commit Message

Max Reitz Feb. 4, 2020, 5:08 p.m. UTC 
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block.c | 62 +++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 40 insertions(+), 22 deletions(-)

Comments

Eric Blake Feb. 5, 2020, 8:56 p.m. UTC | #1 
On 2/4/20 11:08 AM, Max Reitz wrote:
> Signed-off-by: Max Reitz <mreitz@redhat.com>

Rather light on the commit message. But looks like straightforward 
refactoring (with the previous patch making it easier to follow).

> ---
>   block.c | 62 +++++++++++++++++++++++++++++++++++++--------------------
>   1 file changed, 40 insertions(+), 22 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>
Max Reitz Feb. 6, 2020, 11:19 a.m. UTC | #2 
On 05.02.20 21:56, Eric Blake wrote:
> On 2/4/20 11:08 AM, Max Reitz wrote:
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
> 
> Rather light on the commit message. But looks like straightforward
> refactoring (with the previous patch making it easier to follow).

Would this work:

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.

?

Max
Eric Blake Feb. 11, 2020, 3:38 p.m. UTC | #3 
On 2/6/20 5:19 AM, Max Reitz wrote:
> On 05.02.20 21:56, Eric Blake wrote:
>> On 2/4/20 11:08 AM, Max Reitz wrote:
>>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>>
>> Rather light on the commit message. But looks like straightforward
>> refactoring (with the previous patch making it easier to follow).
> 
> Would this work:
> 
> 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.
> 
> ?

Yes.
diff mbox series

Patch

diff --git a/block.c b/block.c
index 39ba6aad0a..b26ec10cce 100644
--- a/block.c
+++ b/block.c
@@ -2280,6 +2280,44 @@  void bdrv_filter_default_perms(BlockDriverState *bs, BdrvChild *c,
     *nshared = (shared & DEFAULT_PERM_PASSTHROUGH) | DEFAULT_PERM_UNCHANGED;
 }
 
+static void bdrv_default_perms_for_backing(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)
+{
+    assert(child_class == &child_backing ||
+           (child_class == &child_of_bds && (role & BDRV_CHILD_COW)));
+
+    /*
+     * We want consistent read from backing files if the parent needs it.
+     * No other operations are performed on backing files.
+     */
+    perm &= BLK_PERM_CONSISTENT_READ;
+
+    /*
+     * If the parent can deal with changing data, we're okay with a
+     * writable and resizable backing file.
+     * TODO Require !(perm & BLK_PERM_CONSISTENT_READ), too?
+     */
+    if (shared & BLK_PERM_WRITE) {
+        shared = BLK_PERM_WRITE | BLK_PERM_RESIZE;
+    } else {
+        shared = 0;
+    }
+
+    shared |= BLK_PERM_CONSISTENT_READ | BLK_PERM_GRAPH_MOD |
+              BLK_PERM_WRITE_UNCHANGED;
+
+    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,
@@ -2317,28 +2355,8 @@  void bdrv_format_default_perms(BlockDriverState *bs, BdrvChild *c,
         *nperm = perm;
         *nshared = shared;
     } else {
-        /* We want consistent read from backing files if the parent needs it.
-         * No other operations are performed on backing files. */
-        perm &= BLK_PERM_CONSISTENT_READ;
-
-        /* If the parent can deal with changing data, we're okay with a
-         * writable and resizable backing file. */
-        /* TODO Require !(perm & BLK_PERM_CONSISTENT_READ), too? */
-        if (shared & BLK_PERM_WRITE) {
-            shared = BLK_PERM_WRITE | BLK_PERM_RESIZE;
-        } else {
-            shared = 0;
-        }
-
-        shared |= BLK_PERM_CONSISTENT_READ | BLK_PERM_GRAPH_MOD |
-                  BLK_PERM_WRITE_UNCHANGED;
-
-        if (bs->open_flags & BDRV_O_INACTIVE) {
-            shared |= BLK_PERM_WRITE | BLK_PERM_RESIZE;
-        }
-
-        *nperm = perm;
-        *nshared = shared;
+        bdrv_default_perms_for_backing(bs, c, child_class, role, reopen_queue,
+                                       perm, shared, nperm, nshared);
     }
 }