[PATCH v2 0/7] block: permission update fix & refactor

[PATCH v2 0/7] block: permission update fix & refactor

[Vladimir Sementsov-Ogievskiy]

Hi all!

These series supersedes "Fix nested permission update" and includes one
more fix (patch 01) and more improvements.

I think patch 01 is good to have in 5.2, 02 is probably OK for 5.2 and
the others are OK for next release. Still all may be taken to 5.2, up to
block maintainers.

Actually the series is a first part of my work announced in
https://lists.gnu.org/archive/html/qemu-devel/2020-09/msg08386.html
to bring correct order to permission update (topological sort),
update permission only on updated graph (and rollback graph changes),
get rid of .active fields in block jobs.

Supersedes: <20201031123502.4558-1-vsementsov@virtuozzo.com>

v2: all new except for 03:, which uses suggestions by Albert and more
strict version of bdrv_replace_node.

Vladimir Sementsov-Ogievskiy (7):
  block: add forgotten bdrv_abort_perm_update() to
    bdrv_co_invalidate_cache()
  block: add bdrv_replace_node_common()
  block: make bdrv_drop_intermediate() less wrong
  block: add bdrv_refresh_perms() helper
  block: bdrv_set_perm() drop redundant parameters.
  block: bdrv_child_set_perm() drop redundant parameters.
  block: drop tighten_restrictions

 block.c | 256 +++++++++++++++++++++++---------------------------------
 1 file changed, 105 insertions(+), 151 deletions(-)

-- 
2.21.3

[PATCH v2 1/7] block: add forgotten bdrv_abort_perm_update() to bdrv_co_invalidate_cache()

[Vladimir Sementsov-Ogievskiy]

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block.c b/block.c
index 56bacc9e9f..19db7b7aeb 100644
--- a/block.c
+++ b/block.c
@@ -5782,6 +5782,7 @@ int coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs, Error **errp)
         bdrv_get_cumulative_perm(bs, &perm, &shared_perm);
         ret = bdrv_check_perm(bs, NULL, perm, shared_perm, NULL, NULL, errp);
         if (ret < 0) {
+            bdrv_abort_perm_update(bs);
             bs->open_flags |= BDRV_O_INACTIVE;
             return ret;
         }
-- 
2.21.3

Re: [PATCH v2 1/7] block: add forgotten bdrv_abort_perm_update() to bdrv_co_invalidate_cache()

[Alberto Garcia]

On Fri 06 Nov 2020 01:42:35 PM CET, Vladimir Sementsov-Ogievskiy wrote:
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto

[PATCH v2 2/7] block: add bdrv_replace_node_common()

[Vladimir Sementsov-Ogievskiy]

Add new parameter to bdrv_replace_node(): auto_skip. With
auto_skip=false we'll have stricter behavior: update _all_ from
parents or fail. New behaviour will be used in the following commit in
block.c, so keep original function name as public interface.

Note: new error message is a bit funny in contrast with further
"Cannot" in case of frozen child, but we'd better keep some difference
to make it possible to distinguish one from another on failure. Still,
actually we'd better refactor should_update_child() call to distinguish
also different kinds of "should not". Let's do it later.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/block.c b/block.c
index 19db7b7aeb..11c862d691 100644
--- a/block.c
+++ b/block.c
@@ -4563,8 +4563,16 @@ static bool should_update_child(BdrvChild *c, BlockDriverState *to)
     return ret;
 }
 
-void bdrv_replace_node(BlockDriverState *from, BlockDriverState *to,
-                       Error **errp)
+/*
+ * With auto_skip=true bdrv_replace_node_common skips updating from parents
+ * if it creates a parent-child relation loop or if parent is block-job.
+ *
+ * With auto_skip=false the error is returned if from has a parent which should
+ * not be updated.
+ */
+static void bdrv_replace_node_common(BlockDriverState *from,
+                                     BlockDriverState *to,
+                                     bool auto_skip, Error **errp)
 {
     BdrvChild *c, *next;
     GSList *list = NULL, *p;
@@ -4583,7 +4591,12 @@ void bdrv_replace_node(BlockDriverState *from, BlockDriverState *to,
     QLIST_FOREACH_SAFE(c, &from->parents, next_parent, next) {
         assert(c->bs == from);
         if (!should_update_child(c, to)) {
-            continue;
+            if (auto_skip) {
+                continue;
+            }
+            error_setg(errp, "Should not change '%s' link to '%s'",
+                       c->name, from->node_name);
+            goto out;
         }
         if (c->frozen) {
             error_setg(errp, "Cannot change '%s' link to '%s'",
@@ -4623,6 +4636,12 @@ out:
     bdrv_unref(from);
 }
 
+void bdrv_replace_node(BlockDriverState *from, BlockDriverState *to,
+                       Error **errp)
+{
+    return bdrv_replace_node_common(from, to, true, errp);
+}
+
 /*
  * Add new bs contents at the top of an image chain while the chain is
  * live, while keeping required fields on the top layer.
-- 
2.21.3

Re: [PATCH v2 2/7] block: add bdrv_replace_node_common()

[Alberto Garcia]

On Fri 06 Nov 2020 01:42:36 PM CET, Vladimir Sementsov-Ogievskiy wrote:
> Add new parameter to bdrv_replace_node(): auto_skip. With
> auto_skip=false we'll have stricter behavior: update _all_ from
> parents or fail. New behaviour will be used in the following commit in
> block.c, so keep original function name as public interface.
>
> Note: new error message is a bit funny in contrast with further
> "Cannot" in case of frozen child, but we'd better keep some difference
> to make it possible to distinguish one from another on failure. Still,
> actually we'd better refactor should_update_child() call to distinguish
> also different kinds of "should not". Let's do it later.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto

[PATCH v2 3/7] block: make bdrv_drop_intermediate() less wrong

[Vladimir Sementsov-Ogievskiy]

First, permission update loop tries to do iterations transactionally,
but the whole update is not transactional: nobody roll-back successful
loop iterations when some iteration fails.

Second, in the iteration we have nested permission update:
c->klass->update_filename may point to bdrv_child_cb_update_filename()
which calls bdrv_backing_update_filename(), which may do node reopen to
RW.

Permission update system is not prepared to nested updates, at least it
has intermediate permission-update state stored in BdrvChild
structures: has_backup_perm, backup_perm and backup_shared_perm.

So, let's first do bdrv_replace_node_common() (which is more
transactional than open-coded update in bdrv_drop_intermediate()) and
then call update_filename() in separate. We still do not rollback
changes in case of update_filename() failure but it's not much worse
than pre-patch behavior.

Note that bdrv_replace_node_common() does check for frozen children,
so corresponding check is dropped in bdrv_drop_intermediate().

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block.c | 54 ++++++++++++++++++++++++------------------------------
 1 file changed, 24 insertions(+), 30 deletions(-)

diff --git a/block.c b/block.c
index 11c862d691..77a3f8f1e2 100644
--- a/block.c
+++ b/block.c
@@ -4910,9 +4910,11 @@ int bdrv_drop_intermediate(BlockDriverState *top, BlockDriverState *base,
 {
     BlockDriverState *explicit_top = top;
     bool update_inherits_from;
-    BdrvChild *c, *next;
+    BdrvChild *c;
     Error *local_err = NULL;
     int ret = -EIO;
+    g_autoptr(GSList) updated_children = NULL;
+    GSList *p;
 
     bdrv_ref(top);
     bdrv_subtree_drained_begin(top);
@@ -4926,14 +4928,6 @@ int bdrv_drop_intermediate(BlockDriverState *top, BlockDriverState *base,
         goto exit;
     }
 
-    /* This function changes all links that point to top and makes
-     * them point to base. Check that none of them is frozen. */
-    QLIST_FOREACH(c, &top->parents, next_parent) {
-        if (c->frozen) {
-            goto exit;
-        }
-    }
-
     /* If 'base' recursively inherits from 'top' then we should set
      * base->inherits_from to top->inherits_from after 'top' and all
      * other intermediate nodes have been dropped.
@@ -4950,36 +4944,36 @@ int bdrv_drop_intermediate(BlockDriverState *top, BlockDriverState *base,
         backing_file_str = base->filename;
     }
 
-    QLIST_FOREACH_SAFE(c, &top->parents, next_parent, next) {
-        /* Check whether we are allowed to switch c from top to base */
-        GSList *ignore_children = g_slist_prepend(NULL, c);
-        ret = bdrv_check_update_perm(base, NULL, c->perm, c->shared_perm,
-                                     ignore_children, NULL, &local_err);
-        g_slist_free(ignore_children);
-        if (ret < 0) {
-            error_report_err(local_err);
-            goto exit;
-        }
+    QLIST_FOREACH(c, &top->parents, next_parent) {
+        updated_children = g_slist_prepend(updated_children, c);
+    }
+
+    bdrv_replace_node_common(top, base, false, &local_err);
+    if (local_err) {
+        error_report_err(local_err);
+        goto exit;
+    }
+
+    for (p = updated_children; p; p = p->next) {
+        c = p->data;
 
-        /* If so, update the backing file path in the image file */
         if (c->klass->update_filename) {
             ret = c->klass->update_filename(c, base, backing_file_str,
                                             &local_err);
             if (ret < 0) {
-                bdrv_abort_perm_update(base);
+                /*
+                 * TODO: Actually, we want to rollback all previous iterations
+                 * of this loop, and (which is almost impossible) previous
+                 * bdrv_replace_node()...
+                 *
+                 * Note, that c->klass->update_filename may lead to permission
+                 * update, so it's a bad idea to call it inside permission
+                 * update transaction of bdrv_replace_node.
+                 */
                 error_report_err(local_err);
                 goto exit;
             }
         }
-
-        /*
-         * Do the actual switch in the in-memory graph.
-         * Completes bdrv_check_update_perm() transaction internally.
-         * c->frozen is false, we have checked that above.
-         */
-        bdrv_ref(base);
-        bdrv_replace_child(c, base);
-        bdrv_unref(top);
     }
 
     if (update_inherits_from) {
-- 
2.21.3

Re: [PATCH v2 3/7] block: make bdrv_drop_intermediate() less wrong

[Alberto Garcia]

On Fri 06 Nov 2020 01:42:37 PM CET, Vladimir Sementsov-Ogievskiy wrote:
> First, permission update loop tries to do iterations transactionally,
> but the whole update is not transactional: nobody roll-back successful
> loop iterations when some iteration fails.
>
> Second, in the iteration we have nested permission update:
> c->klass->update_filename may point to bdrv_child_cb_update_filename()
> which calls bdrv_backing_update_filename(), which may do node reopen to
> RW.
>
> Permission update system is not prepared to nested updates, at least it
> has intermediate permission-update state stored in BdrvChild
> structures: has_backup_perm, backup_perm and backup_shared_perm.
>
> So, let's first do bdrv_replace_node_common() (which is more
> transactional than open-coded update in bdrv_drop_intermediate()) and
> then call update_filename() in separate. We still do not rollback
> changes in case of update_filename() failure but it's not much worse
> than pre-patch behavior.
>
> Note that bdrv_replace_node_common() does check for frozen children,
> so corresponding check is dropped in bdrv_drop_intermediate().
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto

[PATCH v2 4/7] block: add bdrv_refresh_perms() helper

[Vladimir Sementsov-Ogievskiy]

Make separate function for common pattern.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block.c | 60 ++++++++++++++++++++++++++++-----------------------------
 1 file changed, 30 insertions(+), 30 deletions(-)

diff --git a/block.c b/block.c
index 77a3f8f1e2..fc7633307f 100644
--- a/block.c
+++ b/block.c
@@ -2321,6 +2321,23 @@ static void bdrv_child_abort_perm_update(BdrvChild *c)
     bdrv_abort_perm_update(c->bs);
 }
 
+static int bdrv_refresh_perms(BlockDriverState *bs, bool *tighten_restrictions,
+                              Error **errp)
+{
+    int ret;
+    uint64_t perm, shared_perm;
+
+    bdrv_get_cumulative_perm(bs, &perm, &shared_perm);
+    ret = bdrv_check_perm(bs, NULL, perm, shared_perm, NULL, NULL, errp);
+    if (ret < 0) {
+        bdrv_abort_perm_update(bs);
+        return ret;
+    }
+    bdrv_set_perm(bs, perm, shared_perm);
+
+    return 0;
+}
+
 int bdrv_child_try_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared,
                             Error **errp)
 {
@@ -2636,22 +2653,15 @@ static void bdrv_replace_child(BdrvChild *child, BlockDriverState *new_bs)
     }
 
     if (old_bs) {
-        /* Update permissions for old node. This is guaranteed to succeed
-         * because we're just taking a parent away, so we're loosening
-         * restrictions. */
         bool tighten_restrictions;
-        int ret;
 
-        bdrv_get_cumulative_perm(old_bs, &perm, &shared_perm);
-        ret = bdrv_check_perm(old_bs, NULL, perm, shared_perm, NULL,
-                              &tighten_restrictions, NULL);
+        /*
+         * Update permissions for old node. We're just taking a parent away, so
+         * we're loosening restrictions. Errors of permission update are not
+         * fatal in this case, ignore them.
+         */
+        bdrv_refresh_perms(old_bs, &tighten_restrictions, NULL);
         assert(tighten_restrictions == false);
-        if (ret < 0) {
-            /* We only tried to loosen restrictions, so errors are not fatal */
-            bdrv_abort_perm_update(old_bs);
-        } else {
-            bdrv_set_perm(old_bs, perm, shared_perm);
-        }
 
         /* When the parent requiring a non-default AioContext is removed, the
          * node moves back to the main AioContext */
@@ -5760,7 +5770,6 @@ void bdrv_init_with_whitelist(void)
 int coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs, Error **errp)
 {
     BdrvChild *child, *parent;
-    uint64_t perm, shared_perm;
     Error *local_err = NULL;
     int ret;
     BdrvDirtyBitmap *bm;
@@ -5792,14 +5801,11 @@ int coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs, Error **errp)
      */
     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, errp);
+        ret = bdrv_refresh_perms(bs, NULL, errp);
         if (ret < 0) {
-            bdrv_abort_perm_update(bs);
             bs->open_flags |= BDRV_O_INACTIVE;
             return ret;
         }
-        bdrv_set_perm(bs, perm, shared_perm);
 
         if (bs->drv->bdrv_co_invalidate_cache) {
             bs->drv->bdrv_co_invalidate_cache(bs, &local_err);
@@ -5875,7 +5881,6 @@ static int bdrv_inactivate_recurse(BlockDriverState *bs)
 {
     BdrvChild *child, *parent;
     bool tighten_restrictions;
-    uint64_t perm, shared_perm;
     int ret;
 
     if (!bs->drv) {
@@ -5909,18 +5914,13 @@ static int bdrv_inactivate_recurse(BlockDriverState *bs)
 
     bs->open_flags |= BDRV_O_INACTIVE;
 
-    /* Update permissions, they may differ for inactive nodes */
-    bdrv_get_cumulative_perm(bs, &perm, &shared_perm);
-    ret = bdrv_check_perm(bs, NULL, perm, shared_perm, NULL,
-                          &tighten_restrictions, NULL);
+    /*
+     * Update permissions, they may differ for inactive nodes.
+     * We only tried to loosen restrictions, so errors are not fatal, ignore
+     * them.
+     */
+    bdrv_refresh_perms(bs, &tighten_restrictions, NULL);
     assert(tighten_restrictions == false);
-    if (ret < 0) {
-        /* We only tried to loosen restrictions, so errors are not fatal */
-        bdrv_abort_perm_update(bs);
-    } else {
-        bdrv_set_perm(bs, perm, shared_perm);
-    }
-
 
     /* Recursively inactivate children */
     QLIST_FOREACH(child, &bs->children, next) {
-- 
2.21.3

Re: [PATCH v2 4/7] block: add bdrv_refresh_perms() helper

[Alberto Garcia]

On Fri 06 Nov 2020 01:42:38 PM CET, Vladimir Sementsov-Ogievskiy wrote:
> Make separate function for common pattern.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  block.c | 60 ++++++++++++++++++++++++++++-----------------------------
>  1 file changed, 30 insertions(+), 30 deletions(-)
>
> diff --git a/block.c b/block.c
> index 77a3f8f1e2..fc7633307f 100644
> --- a/block.c
> +++ b/block.c
> @@ -2321,6 +2321,23 @@ static void bdrv_child_abort_perm_update(BdrvChild *c)
>      bdrv_abort_perm_update(c->bs);
>  }
>  
> +static int bdrv_refresh_perms(BlockDriverState *bs, bool *tighten_restrictions,
> +                              Error **errp)
> +{
> +    int ret;
> +    uint64_t perm, shared_perm;
> +
> +    bdrv_get_cumulative_perm(bs, &perm, &shared_perm);
> +    ret = bdrv_check_perm(bs, NULL, perm, shared_perm, NULL, NULL,
> errp);

Aren't you supposed to pass tighten_restrictions here ?

Berto

Re: [PATCH v2 4/7] block: add bdrv_refresh_perms() helper

[Vladimir Sementsov-Ogievskiy]

06.11.2020 18:14, Alberto Garcia wrote:
> On Fri 06 Nov 2020 01:42:38 PM CET, Vladimir Sementsov-Ogievskiy wrote:
>> Make separate function for common pattern.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   block.c | 60 ++++++++++++++++++++++++++++-----------------------------
>>   1 file changed, 30 insertions(+), 30 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index 77a3f8f1e2..fc7633307f 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -2321,6 +2321,23 @@ static void bdrv_child_abort_perm_update(BdrvChild *c)
>>       bdrv_abort_perm_update(c->bs);
>>   }
>>   
>> +static int bdrv_refresh_perms(BlockDriverState *bs, bool *tighten_restrictions,
>> +                              Error **errp)
>> +{
>> +    int ret;
>> +    uint64_t perm, shared_perm;
>> +
>> +    bdrv_get_cumulative_perm(bs, &perm, &shared_perm);
>> +    ret = bdrv_check_perm(bs, NULL, perm, shared_perm, NULL, NULL,
>> errp);
> 
> Aren't you supposed to pass tighten_restrictions here ?
> 

Oops, yes I should


-- 
Best regards,
Vladimir

Re: [PATCH v2 4/7] block: add bdrv_refresh_perms() helper

[Vladimir Sementsov-Ogievskiy]

09.11.2020 10:04, Vladimir Sementsov-Ogievskiy wrote:
> 06.11.2020 18:14, Alberto Garcia wrote:
>> On Fri 06 Nov 2020 01:42:38 PM CET, Vladimir Sementsov-Ogievskiy wrote:
>>> Make separate function for common pattern.
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>> ---
>>>   block.c | 60 ++++++++++++++++++++++++++++-----------------------------
>>>   1 file changed, 30 insertions(+), 30 deletions(-)
>>>
>>> diff --git a/block.c b/block.c
>>> index 77a3f8f1e2..fc7633307f 100644
>>> --- a/block.c
>>> +++ b/block.c
>>> @@ -2321,6 +2321,23 @@ static void bdrv_child_abort_perm_update(BdrvChild *c)
>>>       bdrv_abort_perm_update(c->bs);
>>>   }
>>> +static int bdrv_refresh_perms(BlockDriverState *bs, bool *tighten_restrictions,
>>> +                              Error **errp)
>>> +{
>>> +    int ret;
>>> +    uint64_t perm, shared_perm;
>>> +
>>> +    bdrv_get_cumulative_perm(bs, &perm, &shared_perm);
>>> +    ret = bdrv_check_perm(bs, NULL, perm, shared_perm, NULL, NULL,
>>> errp);
>>
>> Aren't you supposed to pass tighten_restrictions here ?
>>
> 
> Oops, yes I should
> 

So, squash-in:


diff --git a/block.c b/block.c
index fc7633307f..a96dc07364 100644
--- a/block.c
+++ b/block.c
@@ -2328,7 +2328,8 @@ static int bdrv_refresh_perms(BlockDriverState *bs, bool *tighten_restrictions,
      uint64_t perm, shared_perm;
  
      bdrv_get_cumulative_perm(bs, &perm, &shared_perm);
-    ret = bdrv_check_perm(bs, NULL, perm, shared_perm, NULL, NULL, errp);
+    ret = bdrv_check_perm(bs, NULL, perm, shared_perm, NULL,
+                          tighten_restrictions, errp);
      if (ret < 0) {
          bdrv_abort_perm_update(bs);
          return ret;



(produces simple conflict when applying "block: drop tighten_restrictions")

-- 
Best regards,
Vladimir

[PATCH v2 5/7] block: bdrv_set_perm() drop redundant parameters.

[Vladimir Sementsov-Ogievskiy]

We should never set permissions other than cumulative permissions of
parents. During bdrv_reopen_multiple() we _check_ for synthetic
permissions but when we do _set_ the graph is already updated.
Add an assertion to bdrv_reopen_multiple(), other cases are more
obvious.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/block.c b/block.c
index fc7633307f..b61d20252f 100644
--- a/block.c
+++ b/block.c
@@ -2106,9 +2106,9 @@ static void bdrv_abort_perm_update(BlockDriverState *bs)
     }
 }
 
-static void bdrv_set_perm(BlockDriverState *bs, uint64_t cumulative_perms,
-                          uint64_t cumulative_shared_perms)
+static void bdrv_set_perm(BlockDriverState *bs)
 {
+    uint64_t cumulative_perms, cumulative_shared_perms;
     BlockDriver *drv = bs->drv;
     BdrvChild *c;
 
@@ -2116,6 +2116,8 @@ static void bdrv_set_perm(BlockDriverState *bs, uint64_t cumulative_perms,
         return;
     }
 
+    bdrv_get_cumulative_perm(bs, &cumulative_perms, &cumulative_shared_perms);
+
     /* Update this node */
     if (drv->bdrv_set_perm) {
         drv->bdrv_set_perm(bs, cumulative_perms, cumulative_shared_perms);
@@ -2298,16 +2300,12 @@ static int bdrv_child_check_perm(BdrvChild *c, BlockReopenQueue *q,
 
 static void bdrv_child_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared)
 {
-    uint64_t cumulative_perms, cumulative_shared_perms;
-
     c->has_backup_perm = false;
 
     c->perm = perm;
     c->shared_perm = shared;
 
-    bdrv_get_cumulative_perm(c->bs, &cumulative_perms,
-                             &cumulative_shared_perms);
-    bdrv_set_perm(c->bs, cumulative_perms, cumulative_shared_perms);
+    bdrv_set_perm(c->bs);
 }
 
 static void bdrv_child_abort_perm_update(BdrvChild *c)
@@ -2333,7 +2331,7 @@ static int bdrv_refresh_perms(BlockDriverState *bs, bool *tighten_restrictions,
         bdrv_abort_perm_update(bs);
         return ret;
     }
-    bdrv_set_perm(bs, perm, shared_perm);
+    bdrv_set_perm(bs);
 
     return 0;
 }
@@ -2634,7 +2632,6 @@ static void bdrv_replace_child_noperm(BdrvChild *child,
 static void bdrv_replace_child(BdrvChild *child, BlockDriverState *new_bs)
 {
     BlockDriverState *old_bs = child->bs;
-    uint64_t perm, shared_perm;
 
     /* Asserts that child->frozen == false */
     bdrv_replace_child_noperm(child, new_bs);
@@ -2648,8 +2645,7 @@ static void bdrv_replace_child(BdrvChild *child, BlockDriverState *new_bs)
      * restrictions.
      */
     if (new_bs) {
-        bdrv_get_cumulative_perm(new_bs, &perm, &shared_perm);
-        bdrv_set_perm(new_bs, perm, shared_perm);
+        bdrv_set_perm(new_bs);
     }
 
     if (old_bs) {
@@ -3867,7 +3863,13 @@ cleanup_perm:
         }
 
         if (ret == 0) {
-            bdrv_set_perm(state->bs, state->perm, state->shared_perm);
+            uint64_t perm, shared;
+
+            bdrv_get_cumulative_perm(state->bs, &perm, &shared);
+            assert(perm == state->perm);
+            assert(shared == state->shared_perm);
+
+            bdrv_set_perm(state->bs);
         } else {
             bdrv_abort_perm_update(state->bs);
             if (state->replace_backing_bs && state->new_backing_bs) {
@@ -4637,8 +4639,7 @@ static void bdrv_replace_node_common(BlockDriverState *from,
         bdrv_unref(from);
     }
 
-    bdrv_get_cumulative_perm(to, &perm, &shared);
-    bdrv_set_perm(to, perm, shared);
+    bdrv_set_perm(to);
 
 out:
     g_slist_free(list);
-- 
2.21.3

Re: [PATCH v2 5/7] block: bdrv_set_perm() drop redundant parameters.

[Max Reitz]

On 06.11.20 13:42, Vladimir Sementsov-Ogievskiy wrote:
> We should never set permissions other than cumulative permissions of
> parents. During bdrv_reopen_multiple() we _check_ for synthetic
> permissions but when we do _set_ the graph is already updated.
> Add an assertion to bdrv_reopen_multiple(), other cases are more
> obvious.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   block.c | 29 +++++++++++++++--------------
>   1 file changed, 15 insertions(+), 14 deletions(-)

(Perhaps bdrv_commit_perm() might be a better name then, but I’m afraid 
such a name change might be quite invasive (because AFAIR *_set_perm is 
used quite often).)

Reviewed-by: Max Reitz <mreitz@redhat.com>

Re: [PATCH v2 5/7] block: bdrv_set_perm() drop redundant parameters.

[Vladimir Sementsov-Ogievskiy]

09.11.2020 15:20, Max Reitz wrote:
> On 06.11.20 13:42, Vladimir Sementsov-Ogievskiy wrote:
>> We should never set permissions other than cumulative permissions of
>> parents. During bdrv_reopen_multiple() we _check_ for synthetic
>> permissions but when we do _set_ the graph is already updated.
>> Add an assertion to bdrv_reopen_multiple(), other cases are more
>> obvious.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   block.c | 29 +++++++++++++++--------------
>>   1 file changed, 15 insertions(+), 14 deletions(-)
> 
> (Perhaps bdrv_commit_perm() might be a better name then, but I’m afraid such a name change might be quite invasive (because AFAIR *_set_perm is used quite often).)
> 
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> 

Thanks for reviewing!

Actually, I plan to split and organize in a similar transactional way graph-update operations:

  - aio context change
  - child replacement
  - permission update

So, we'll have a chance to discuss final names later. I think about prepare/commit/abort too, as it is more common than check/set/abort. Also, check now actually do set permissions in BdrvChild, so it isn't "just check" (and the fact that we should do "abort" after "check" was always a bit odd).

-- 
Best regards,
Vladimir

[PATCH v2 6/7] block: bdrv_child_set_perm() drop redundant parameters.

[Vladimir Sementsov-Ogievskiy]

We must set the permission used for _check_.  Assert that we have
backup and drop extra arguments.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/block.c b/block.c
index b61d20252f..b44db05d14 100644
--- a/block.c
+++ b/block.c
@@ -1897,7 +1897,7 @@ static int bdrv_child_check_perm(BdrvChild *c, BlockReopenQueue *q,
                                  GSList *ignore_children,
                                  bool *tighten_restrictions, Error **errp);
 static void bdrv_child_abort_perm_update(BdrvChild *c);
-static void bdrv_child_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared);
+static void bdrv_child_set_perm(BdrvChild *c);
 
 typedef struct BlockReopenQueueEntry {
      bool prepared;
@@ -2131,11 +2131,7 @@ static void bdrv_set_perm(BlockDriverState *bs)
 
     /* Update all children */
     QLIST_FOREACH(c, &bs->children, next) {
-        uint64_t cur_perm, cur_shared;
-        bdrv_child_perm(bs, c->bs, c, c->role, NULL,
-                        cumulative_perms, cumulative_shared_perms,
-                        &cur_perm, &cur_shared);
-        bdrv_child_set_perm(c, cur_perm, cur_shared);
+        bdrv_child_set_perm(c);
     }
 }
 
@@ -2298,13 +2294,10 @@ static int bdrv_child_check_perm(BdrvChild *c, BlockReopenQueue *q,
     return 0;
 }
 
-static void bdrv_child_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared)
+static void bdrv_child_set_perm(BdrvChild *c)
 {
     c->has_backup_perm = false;
 
-    c->perm = perm;
-    c->shared_perm = shared;
-
     bdrv_set_perm(c->bs);
 }
 
@@ -2362,7 +2355,7 @@ int bdrv_child_try_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared,
         return ret;
     }
 
-    bdrv_child_set_perm(c, perm, shared);
+    bdrv_child_set_perm(c);
 
     return 0;
 }
-- 
2.21.3

Re: [PATCH v2 6/7] block: bdrv_child_set_perm() drop redundant parameters.

[Max Reitz]

On 06.11.20 13:42, Vladimir Sementsov-Ogievskiy wrote:
> We must set the permission used for _check_.  Assert that we have
> backup and drop extra arguments.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   block.c | 15 ++++-----------
>   1 file changed, 4 insertions(+), 11 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>

[PATCH v2 7/7] block: drop tighten_restrictions

[Vladimir Sementsov-Ogievskiy]

The only users of this thing are:
 1. bdrv_child_try_set_perm, to ignore failures on loosen restrictions
 2. assertion in bdrv_replace_child
 3. assertion in bdrv_inactivate_recurse

Assertions are not enough reason for overcomplication the permission
update system. So, look at bdrv_child_try_set_perm.

We are interested in tighten_restrictions only on failure. But on
failure this field is not reliable: we may fail in the middle of
permission update, some nodes are not touched and we don't know should
their permissions be tighten or not. So, we rely on the fact that if we
loose restrictions on some node (or BdrvChild), we'll not tighten
restriction in the whole subtree as part of this update (assertions 2
and 3 rely on this fact as well). And, if we rely on this fact anyway,
we can just check it on top, and don't pass additional pointer through
the whole recursive infrastructure.

Note also, that further patches will fix real bugs in permission update
system, so now is good time to simplify it, as a help for further
refactorings.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block.c | 88 +++++++++++----------------------------------------------
 1 file changed, 17 insertions(+), 71 deletions(-)

diff --git a/block.c b/block.c
index b44db05d14..8437d579e0 100644
--- a/block.c
+++ b/block.c
@@ -1894,8 +1894,7 @@ static int bdrv_fill_options(QDict **options, const char *filename,
 
 static int bdrv_child_check_perm(BdrvChild *c, BlockReopenQueue *q,
                                  uint64_t perm, uint64_t shared,
-                                 GSList *ignore_children,
-                                 bool *tighten_restrictions, Error **errp);
+                                 GSList *ignore_children, Error **errp);
 static void bdrv_child_abort_perm_update(BdrvChild *c);
 static void bdrv_child_set_perm(BdrvChild *c);
 
@@ -1968,43 +1967,18 @@ static void bdrv_child_perm(BlockDriverState *bs, BlockDriverState *child_bs,
  * permissions of all its parents. This involves checking whether all necessary
  * permission changes to child nodes can be performed.
  *
- * Will set *tighten_restrictions to true if and only if new permissions have to
- * be taken or currently shared permissions are to be unshared.  Otherwise,
- * errors are not fatal as long as the caller accepts that the restrictions
- * remain tighter than they need to be.  The caller still has to abort the
- * transaction.
- * @tighten_restrictions cannot be used together with @q: When reopening, we may
- * encounter fatal errors even though no restrictions are to be tightened.  For
- * example, changing a node from RW to RO will fail if the WRITE permission is
- * to be kept.
- *
  * A call to this function must always be followed by a call to bdrv_set_perm()
  * or bdrv_abort_perm_update().
  */
 static int bdrv_check_perm(BlockDriverState *bs, BlockReopenQueue *q,
                            uint64_t cumulative_perms,
                            uint64_t cumulative_shared_perms,
-                           GSList *ignore_children,
-                           bool *tighten_restrictions, Error **errp)
+                           GSList *ignore_children, Error **errp)
 {
     BlockDriver *drv = bs->drv;
     BdrvChild *c;
     int ret;
 
-    assert(!q || !tighten_restrictions);
-
-    if (tighten_restrictions) {
-        uint64_t current_perms, current_shared;
-        uint64_t added_perms, removed_shared_perms;
-
-        bdrv_get_cumulative_perm(bs, &current_perms, &current_shared);
-
-        added_perms = cumulative_perms & ~current_perms;
-        removed_shared_perms = current_shared & ~cumulative_shared_perms;
-
-        *tighten_restrictions = added_perms || removed_shared_perms;
-    }
-
     /* Write permissions never work with read-only images */
     if ((cumulative_perms & (BLK_PERM_WRITE | BLK_PERM_WRITE_UNCHANGED)) &&
         !bdrv_is_writable_after_reopen(bs, q))
@@ -2061,18 +2035,12 @@ static int bdrv_check_perm(BlockDriverState *bs, BlockReopenQueue *q,
     /* Check all children */
     QLIST_FOREACH(c, &bs->children, next) {
         uint64_t cur_perm, cur_shared;
-        bool child_tighten_restr;
 
         bdrv_child_perm(bs, c->bs, c, c->role, q,
                         cumulative_perms, cumulative_shared_perms,
                         &cur_perm, &cur_shared);
         ret = bdrv_child_check_perm(c, q, cur_perm, cur_shared, ignore_children,
-                                    tighten_restrictions ? &child_tighten_restr
-                                                         : NULL,
                                     errp);
-        if (tighten_restrictions) {
-            *tighten_restrictions |= child_tighten_restr;
-        }
         if (ret < 0) {
             return ret;
         }
@@ -2195,22 +2163,18 @@ char *bdrv_perm_names(uint64_t perm)
  * set, the BdrvChild objects in this list are ignored in the calculations;
  * this allows checking permission updates for an existing reference.
  *
- * See bdrv_check_perm() for the semantics of @tighten_restrictions.
- *
  * Needs to be followed by a call to either bdrv_set_perm() or
  * bdrv_abort_perm_update(). */
 static int bdrv_check_update_perm(BlockDriverState *bs, BlockReopenQueue *q,
                                   uint64_t new_used_perm,
                                   uint64_t new_shared_perm,
                                   GSList *ignore_children,
-                                  bool *tighten_restrictions,
                                   Error **errp)
 {
     BdrvChild *c;
     uint64_t cumulative_perms = new_used_perm;
     uint64_t cumulative_shared_perms = new_shared_perm;
 
-    assert(!q || !tighten_restrictions);
 
     /* There is no reason why anyone couldn't tolerate write_unchanged */
     assert(new_shared_perm & BLK_PERM_WRITE_UNCHANGED);
@@ -2224,10 +2188,6 @@ static int bdrv_check_update_perm(BlockDriverState *bs, BlockReopenQueue *q,
             char *user = bdrv_child_user_desc(c);
             char *perm_names = bdrv_perm_names(new_used_perm & ~c->shared_perm);
 
-            if (tighten_restrictions) {
-                *tighten_restrictions = true;
-            }
-
             error_setg(errp, "Conflicts with use by %s as '%s', which does not "
                              "allow '%s' on %s",
                        user, c->name, perm_names, bdrv_get_node_name(c->bs));
@@ -2240,10 +2200,6 @@ static int bdrv_check_update_perm(BlockDriverState *bs, BlockReopenQueue *q,
             char *user = bdrv_child_user_desc(c);
             char *perm_names = bdrv_perm_names(c->perm & ~new_shared_perm);
 
-            if (tighten_restrictions) {
-                *tighten_restrictions = true;
-            }
-
             error_setg(errp, "Conflicts with use by %s as '%s', which uses "
                              "'%s' on %s",
                        user, c->name, perm_names, bdrv_get_node_name(c->bs));
@@ -2257,21 +2213,19 @@ static int bdrv_check_update_perm(BlockDriverState *bs, BlockReopenQueue *q,
     }
 
     return bdrv_check_perm(bs, q, cumulative_perms, cumulative_shared_perms,
-                           ignore_children, tighten_restrictions, errp);
+                           ignore_children, errp);
 }
 
 /* Needs to be followed by a call to either bdrv_child_set_perm() or
  * bdrv_child_abort_perm_update(). */
 static int bdrv_child_check_perm(BdrvChild *c, BlockReopenQueue *q,
                                  uint64_t perm, uint64_t shared,
-                                 GSList *ignore_children,
-                                 bool *tighten_restrictions, Error **errp)
+                                 GSList *ignore_children, Error **errp)
 {
     int ret;
 
     ignore_children = g_slist_prepend(g_slist_copy(ignore_children), c);
-    ret = bdrv_check_update_perm(c->bs, q, perm, shared, ignore_children,
-                                 tighten_restrictions, errp);
+    ret = bdrv_check_update_perm(c->bs, q, perm, shared, ignore_children, errp);
     g_slist_free(ignore_children);
 
     if (ret < 0) {
@@ -2312,14 +2266,13 @@ static void bdrv_child_abort_perm_update(BdrvChild *c)
     bdrv_abort_perm_update(c->bs);
 }
 
-static int bdrv_refresh_perms(BlockDriverState *bs, bool *tighten_restrictions,
-                              Error **errp)
+static int bdrv_refresh_perms(BlockDriverState *bs, Error **errp)
 {
     int ret;
     uint64_t perm, shared_perm;
 
     bdrv_get_cumulative_perm(bs, &perm, &shared_perm);
-    ret = bdrv_check_perm(bs, NULL, perm, shared_perm, NULL, NULL, errp);
+    ret = bdrv_check_perm(bs, NULL, perm, shared_perm, NULL, errp);
     if (ret < 0) {
         bdrv_abort_perm_update(bs);
         return ret;
@@ -2334,13 +2287,12 @@ int bdrv_child_try_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared,
 {
     Error *local_err = NULL;
     int ret;
-    bool tighten_restrictions;
 
-    ret = bdrv_child_check_perm(c, NULL, perm, shared, NULL,
-                                &tighten_restrictions, &local_err);
+    ret = bdrv_child_check_perm(c, NULL, perm, shared, NULL, &local_err);
     if (ret < 0) {
         bdrv_child_abort_perm_update(c);
-        if (tighten_restrictions) {
+        if ((perm & ~c->perm) || (c->shared_perm & ~shared)) {
+            /* tighten permissions */
             error_propagate(errp, local_err);
         } else {
             /*
@@ -2642,15 +2594,12 @@ static void bdrv_replace_child(BdrvChild *child, BlockDriverState *new_bs)
     }
 
     if (old_bs) {
-        bool tighten_restrictions;
-
         /*
          * Update permissions for old node. We're just taking a parent away, so
          * we're loosening restrictions. Errors of permission update are not
          * fatal in this case, ignore them.
          */
-        bdrv_refresh_perms(old_bs, &tighten_restrictions, NULL);
-        assert(tighten_restrictions == false);
+        bdrv_refresh_perms(old_bs, NULL);
 
         /* When the parent requiring a non-default AioContext is removed, the
          * node moves back to the main AioContext */
@@ -2680,8 +2629,7 @@ BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs,
     Error *local_err = NULL;
     int ret;
 
-    ret = bdrv_check_update_perm(child_bs, NULL, perm, shared_perm, NULL, NULL,
-                                 errp);
+    ret = bdrv_check_update_perm(child_bs, NULL, perm, shared_perm, NULL, errp);
     if (ret < 0) {
         bdrv_abort_perm_update(child_bs);
         bdrv_unref(child_bs);
@@ -3813,7 +3761,7 @@ int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp)
     QTAILQ_FOREACH(bs_entry, bs_queue, entry) {
         BDRVReopenState *state = &bs_entry->state;
         ret = bdrv_check_perm(state->bs, bs_queue, state->perm,
-                              state->shared_perm, NULL, NULL, errp);
+                              state->shared_perm, NULL, errp);
         if (ret < 0) {
             goto cleanup_perm;
         }
@@ -3825,7 +3773,7 @@ int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp)
                             bs_queue, state->perm, state->shared_perm,
                             &nperm, &nshared);
             ret = bdrv_check_update_perm(state->new_backing_bs, NULL,
-                                         nperm, nshared, NULL, NULL, errp);
+                                         nperm, nshared, NULL, errp);
             if (ret < 0) {
                 goto cleanup_perm;
             }
@@ -4615,7 +4563,7 @@ static void bdrv_replace_node_common(BlockDriverState *from,
 
     /* Check whether the required permissions can be granted on @to, ignoring
      * all BdrvChild in @list so that they can't block themselves. */
-    ret = bdrv_check_update_perm(to, NULL, perm, shared, list, NULL, errp);
+    ret = bdrv_check_update_perm(to, NULL, perm, shared, list, errp);
     if (ret < 0) {
         bdrv_abort_perm_update(to);
         goto out;
@@ -5795,7 +5743,7 @@ int coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs, Error **errp)
      */
     if (bs->open_flags & BDRV_O_INACTIVE) {
         bs->open_flags &= ~BDRV_O_INACTIVE;
-        ret = bdrv_refresh_perms(bs, NULL, errp);
+        ret = bdrv_refresh_perms(bs, errp);
         if (ret < 0) {
             bs->open_flags |= BDRV_O_INACTIVE;
             return ret;
@@ -5874,7 +5822,6 @@ static bool bdrv_has_bds_parent(BlockDriverState *bs, bool only_active)
 static int bdrv_inactivate_recurse(BlockDriverState *bs)
 {
     BdrvChild *child, *parent;
-    bool tighten_restrictions;
     int ret;
 
     if (!bs->drv) {
@@ -5913,8 +5860,7 @@ static int bdrv_inactivate_recurse(BlockDriverState *bs)
      * We only tried to loosen restrictions, so errors are not fatal, ignore
      * them.
      */
-    bdrv_refresh_perms(bs, &tighten_restrictions, NULL);
-    assert(tighten_restrictions == false);
+    bdrv_refresh_perms(bs, NULL);
 
     /* Recursively inactivate children */
     QLIST_FOREACH(child, &bs->children, next) {
-- 
2.21.3

Re: [PATCH v2 7/7] block: drop tighten_restrictions

[Max Reitz]

On 06.11.20 13:42, Vladimir Sementsov-Ogievskiy wrote:
> The only users of this thing are:
>   1. bdrv_child_try_set_perm, to ignore failures on loosen restrictions
>   2. assertion in bdrv_replace_child
>   3. assertion in bdrv_inactivate_recurse
> 
> Assertions are not enough reason for overcomplication the permission
> update system. So, look at bdrv_child_try_set_perm.
> 
> We are interested in tighten_restrictions only on failure. But on
> failure this field is not reliable: we may fail in the middle of
> permission update, some nodes are not touched and we don't know should
> their permissions be tighten or not. So, we rely on the fact that if we
> loose restrictions on some node (or BdrvChild), we'll not tighten
> restriction in the whole subtree as part of this update (assertions 2
> and 3 rely on this fact as well). And, if we rely on this fact anyway,
> we can just check it on top, and don't pass additional pointer through
> the whole recursive infrastructure.
> 
> Note also, that further patches will fix real bugs in permission update
> system, so now is good time to simplify it, as a help for further
> refactorings.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   block.c | 88 +++++++++++----------------------------------------------
>   1 file changed, 17 insertions(+), 71 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>

Re: [PATCH v2 0/7] block: permission update fix & refactor

[Max Reitz]

On 06.11.20 13:42, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
> 
> These series supersedes "Fix nested permission update" and includes one
> more fix (patch 01) and more improvements.
> 
> I think patch 01 is good to have in 5.2, 02 is probably OK for 5.2 and
> the others are OK for next release. Still all may be taken to 5.2, up to
> block maintainers.
> 
> Actually the series is a first part of my work announced in
> https://lists.gnu.org/archive/html/qemu-devel/2020-09/msg08386.html
> to bring correct order to permission update (topological sort),
> update permission only on updated graph (and rollback graph changes),
> get rid of .active fields in block jobs.
> 
> Supersedes: <20201031123502.4558-1-vsementsov@virtuozzo.com>
> 
> v2: all new except for 03:, which uses suggestions by Albert and more
> strict version of bdrv_replace_node.

Thanks, I took 1 through 3 to my block branch:

https://git.xanclic.moe/XanClic/qemu/commits/branch/block

and 4 through 7 to my block-next branch (squashing in the fix for 4, and 
fixing up the resulting conflict in 7):

https://git.xanclic.moe/XanClic/qemu/commits/branch/block-next

Max

Re: [PATCH v2 0/7] block: permission update fix & refactor

[Vladimir Sementsov-Ogievskiy]

09.11.2020 17:41, Max Reitz wrote:
> On 06.11.20 13:42, Vladimir Sementsov-Ogievskiy wrote:
>> Hi all!
>>
>> These series supersedes "Fix nested permission update" and includes one
>> more fix (patch 01) and more improvements.
>>
>> I think patch 01 is good to have in 5.2, 02 is probably OK for 5.2 and
>> the others are OK for next release. Still all may be taken to 5.2, up to
>> block maintainers.
>>
>> Actually the series is a first part of my work announced in
>> https://lists.gnu.org/archive/html/qemu-devel/2020-09/msg08386.html
>> to bring correct order to permission update (topological sort),
>> update permission only on updated graph (and rollback graph changes),
>> get rid of .active fields in block jobs.
>>
>> Supersedes: <20201031123502.4558-1-vsementsov@virtuozzo.com>
>>
>> v2: all new except for 03:, which uses suggestions by Albert and more
>> strict version of bdrv_replace_node.
> 
> Thanks, I took 1 through 3 to my block branch:
> 
> https://git.xanclic.moe/XanClic/qemu/commits/branch/block
> 
> and 4 through 7 to my block-next branch (squashing in the fix for 4, and fixing up the resulting conflict in 7):
> 
> https://git.xanclic.moe/XanClic/qemu/commits/branch/block-next
> 
> Max
> 

Thank you!

-- 
Best regards,
Vladimir