Hi all!

Finally, I finished v3. Phew.

Missed a soft-freeze. Should we consider it a bugfix? There are bugfixes
here but they are mostly theoretical. So, up to Kevin, should it go to
current release or to the next..

The main point of the series is fixing some permission update problems
(see patches 01-03 as examples), that in turn makes possible more clean
inserting and removing of filters (see patch 26 where .active field is
dropped finally from backup-top filter, we don't need a workaround
anymore).

The series brings util/transactions.c (patch 10) and use of it in
block.c, which allows clean block graph change transactions, with
possibility of reverting all modifications (movement and removement of
children, changing aio context, changing permissions) in reverse order
on failure path.

The series also helps Alberto's "Allow changing bs->file on reopen"
which we want to merge prior dropping x- prefix from blockdev-reopen
command.

v3:
git: https://src.openvz.org/scm/~vsementsov/qemu.git
tag: up-block-topologic-perm-v3

01: move bdrv_ref(), add comment, add missed bdrv_unref(), update copyright msg
02: move bdrv_ref(), add comments, add assertions and missed bdrv_unref()
03: new
04: drop questionable comment, rebased
07: add r-bs
08: improve wording, add more comments
    refactor to use explicit Transaction structure type and typed list
    add note in MAINTAINERS
09: renamed, was "[PATCH v2 11/36] block: bdrv_refresh_perms: check parents compliance"
    function renamed to bdrv_parent_perms_conflict() (and return value is the opposite)
12: add Alberto's r-b
13: check outer condition in outer loop, add comments
17: rebased, fixup of another patch is moved out
18: avoid forward declaration
    keep old logic for error path in bdrv_attach_child_common()
20: rebased, now EINVAL should be first error in new bdrv_replace_node_noperm()
21: rebased, add comments, add missed call to bdrv_refresh_limits()
    new test append-greedy-filter(patch 03) becomes enabled here
22: was "[PATCH v2 24/36] block: add bdrv_remove_backing transaction action"
    Rewrite to support filter child
23: improve comment, add assertion, support removing file-child-based filters
24: rebased. It now effectively reverts 705dde27c6c53b73.
26: new
27: new
28: rewrite with more clean transaction actions (no nested transaction!) and closer to original logic
29: new, instead of worse "[PATCH v2 29/36] blockdev: qmp_x_blockdev_reopen: acquire all contexts"
30: remove unused fields (perm, shared_perm)
    keep "ret = -1" at top, set it to 0 before "goto cleanup"
    add comments
36: update error message, update comment

Vladimir Sementsov-Ogievskiy (36):
  tests/test-bdrv-graph-mod: add test_parallel_exclusive_write
  tests/test-bdrv-graph-mod: add test_parallel_perm_update
  tests/test-bdrv-graph-mod: add test_append_greedy_filter
  block: bdrv_append(): don't consume reference
  block: BdrvChildClass: add .get_parent_aio_context handler
  block: drop ctx argument from bdrv_root_attach_child
  block: make bdrv_reopen_{prepare,commit,abort} private
  util: add transactions.c
  block: bdrv_refresh_perms: check for parents permissions conflict
  block: refactor bdrv_child* permission functions
  block: rewrite bdrv_child_try_set_perm() using bdrv_refresh_perms()
  block: inline bdrv_child_*() permission functions calls
  block: use topological sort for permission update
  block: add bdrv_drv_set_perm transaction action
  block: add bdrv_list_* permission update functions
  block: add bdrv_replace_child_safe() transaction action
  block: fix bdrv_replace_node_common
  block: add bdrv_attach_child_common() transaction action
  block: add bdrv_attach_child_noperm() transaction action
  block: split out bdrv_replace_node_noperm()
  block: adapt bdrv_append() for inserting filters
  block: add bdrv_remove_filter_or_cow transaction action
  block: introduce bdrv_drop_filter()
  block/backup-top: drop .active
  block: drop ignore_children for permission update functions
  block: make bdrv_unset_inherits_from to be a transaction action
  block: make bdrv_refresh_limits() to be a transaction action
  block: add bdrv_set_backing_noperm() transaction action
  block: bdrv_reopen_multiple(): move bdrv_flush to separate pre-prepare
  block: bdrv_reopen_multiple: refresh permissions on updated graph
  block: drop unused permission update functions
  block: inline bdrv_check_perm_common()
  block: inline bdrv_replace_child()
  block: refactor bdrv_child_set_perm_safe() transaction action
  block: rename bdrv_replace_child_safe() to bdrv_replace_child()
  block: refactor bdrv_node_check_perm()

 include/block/block.h            |   13 +-
 include/block/block_int.h        |    8 +-
 include/qemu/transactions.h      |   63 ++
 block.c                          | 1322 ++++++++++++++++++------------
 block/backup-top.c               |   48 +-
 block/block-backend.c            |   13 +-
 block/commit.c                   |    1 +
 block/file-posix.c               |   91 +-
 block/io.c                       |   31 +-
 block/mirror.c                   |    3 -
 blockdev.c                       |    4 -
 blockjob.c                       |   11 +-
 tests/unit/test-bdrv-drain.c     |    2 +-
 tests/unit/test-bdrv-graph-mod.c |  208 ++++-
 util/transactions.c              |   96 +++
 MAINTAINERS                      |    6 +
 tests/qemu-iotests/245           |    2 +-
 tests/qemu-iotests/283.out       |    2 +-
 util/meson.build                 |    1 +
 19 files changed, 1252 insertions(+), 673 deletions(-)
 create mode 100644 include/qemu/transactions.h
 create mode 100644 util/transactions.c

-- 
2.29.2

Patchew URL: https://patchew.org/QEMU/20210317143529.615584-1-vsementsov@virtuozzo.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20210317143529.615584-1-vsementsov@virtuozzo.com
Subject: [PATCH v3 00/36] block: update graph permissions update

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20210317143529.615584-1-vsementsov@virtuozzo.com -> patchew/20210317143529.615584-1-vsementsov@virtuozzo.com
Switched to a new branch 'test'
ac26c21 block: refactor bdrv_node_check_perm()
7171293 block: rename bdrv_replace_child_safe() to bdrv_replace_child()
9430fba block: refactor bdrv_child_set_perm_safe() transaction action
2386ee1 block: inline bdrv_replace_child()
41c9fc1 block: inline bdrv_check_perm_common()
0d06ade block: drop unused permission update functions
72e8fe6 block: bdrv_reopen_multiple: refresh permissions on updated graph
b7f3a63 block: bdrv_reopen_multiple(): move bdrv_flush to separate pre-prepare
b9278b8 block: add bdrv_set_backing_noperm() transaction action
44f6e69 block: make bdrv_refresh_limits() to be a transaction action
a671d58 block: make bdrv_unset_inherits_from to be a transaction action
aa9e316 block: drop ignore_children for permission update functions
6e17bb7 block/backup-top: drop .active
7292e37 block: introduce bdrv_drop_filter()
9e7ce4f block: add bdrv_remove_filter_or_cow transaction action
a5f3a81 block: adapt bdrv_append() for inserting filters
ba82bea block: split out bdrv_replace_node_noperm()
4c97817 block: add bdrv_attach_child_noperm() transaction action
4a4e038 block: add bdrv_attach_child_common() transaction action
a864516 block: fix bdrv_replace_node_common
ce2b5ee block: add bdrv_replace_child_safe() transaction action
0b5ce80 block: add bdrv_list_* permission update functions
5380bbb block: add bdrv_drv_set_perm transaction action
965e80a block: use topological sort for permission update
35d94b7 block: inline bdrv_child_*() permission functions calls
84f3087 block: rewrite bdrv_child_try_set_perm() using bdrv_refresh_perms()
9eb28b7 block: refactor bdrv_child* permission functions
0c068cb block: bdrv_refresh_perms: check for parents permissions conflict
23d55e7 util: add transactions.c
68189c0 block: make bdrv_reopen_{prepare, commit, abort} private
5780b80 block: drop ctx argument from bdrv_root_attach_child
34f4c11 block: BdrvChildClass: add .get_parent_aio_context handler
e79d608 block: bdrv_append(): don't consume reference
87c292c tests/test-bdrv-graph-mod: add test_append_greedy_filter
381fa0c tests/test-bdrv-graph-mod: add test_parallel_perm_update
6df2206 tests/test-bdrv-graph-mod: add test_parallel_exclusive_write

=== OUTPUT BEGIN ===
1/36 Checking commit 6df2206946ee (tests/test-bdrv-graph-mod: add test_parallel_exclusive_write)
2/36 Checking commit 381fa0c5db90 (tests/test-bdrv-graph-mod: add test_parallel_perm_update)
3/36 Checking commit 87c292ca6696 (tests/test-bdrv-graph-mod: add test_append_greedy_filter)
4/36 Checking commit e79d608e8fad (block: bdrv_append(): don't consume reference)
5/36 Checking commit 34f4c11ddd7e (block: BdrvChildClass: add .get_parent_aio_context handler)
6/36 Checking commit 5780b805277e (block: drop ctx argument from bdrv_root_attach_child)
7/36 Checking commit 68189c099a3a (block: make bdrv_reopen_{prepare, commit, abort} private)
ERROR: Author email address is mangled by the mailing list
#2: 
Author: Vladimir Sementsov-Ogievskiy via <qemu-devel@nongnu.org>

total: 1 errors, 0 warnings, 47 lines checked

Patch 7/36 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

8/36 Checking commit 23d55e73e0fa (util: add transactions.c)
9/36 Checking commit 0c068cb2d8be (block: bdrv_refresh_perms: check for parents permissions conflict)
10/36 Checking commit 9eb28b73ad7b (block: refactor bdrv_child* permission functions)
11/36 Checking commit 84f3087f8300 (block: rewrite bdrv_child_try_set_perm() using bdrv_refresh_perms())
12/36 Checking commit 35d94b79e054 (block: inline bdrv_child_*() permission functions calls)
13/36 Checking commit 965e80af7b29 (block: use topological sort for permission update)
14/36 Checking commit 5380bbbb08b6 (block: add bdrv_drv_set_perm transaction action)
15/36 Checking commit 0b5ce8030a8f (block: add bdrv_list_* permission update functions)
16/36 Checking commit ce2b5ee8b20f (block: add bdrv_replace_child_safe() transaction action)
17/36 Checking commit a864516ae0f0 (block: fix bdrv_replace_node_common)
18/36 Checking commit 4a4e0386113f (block: add bdrv_attach_child_common() transaction action)
19/36 Checking commit 4c97817d23f8 (block: add bdrv_attach_child_noperm() transaction action)
20/36 Checking commit ba82bea53db2 (block: split out bdrv_replace_node_noperm())
21/36 Checking commit a5f3a81b5ad0 (block: adapt bdrv_append() for inserting filters)
22/36 Checking commit 9e7ce4f4dea2 (block: add bdrv_remove_filter_or_cow transaction action)
23/36 Checking commit 7292e37fb647 (block: introduce bdrv_drop_filter())
24/36 Checking commit 6e17bb7b248a (block/backup-top: drop .active)
25/36 Checking commit aa9e316c665f (block: drop ignore_children for permission update functions)
26/36 Checking commit a671d58d46da (block: make bdrv_unset_inherits_from to be a transaction action)
27/36 Checking commit 44f6e69305b1 (block: make bdrv_refresh_limits() to be a transaction action)
28/36 Checking commit b9278b83c02b (block: add bdrv_set_backing_noperm() transaction action)
29/36 Checking commit b7f3a63e844d (block: bdrv_reopen_multiple(): move bdrv_flush to separate pre-prepare)
30/36 Checking commit 72e8fe6a54ba (block: bdrv_reopen_multiple: refresh permissions on updated graph)
31/36 Checking commit 0d06ade388bb (block: drop unused permission update functions)
32/36 Checking commit 41c9fc136083 (block: inline bdrv_check_perm_common())
33/36 Checking commit 2386ee1e39eb (block: inline bdrv_replace_child())
34/36 Checking commit 9430fba2337b (block: refactor bdrv_child_set_perm_safe() transaction action)
35/36 Checking commit 71712939d665 (block: rename bdrv_replace_child_safe() to bdrv_replace_child())
36/36 Checking commit ac26c210743e (block: refactor bdrv_node_check_perm())
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20210317143529.615584-1-vsementsov@virtuozzo.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

17.03.2021 18:21, no-reply@patchew.org wrote:
> Patchew URL: https://patchew.org/QEMU/20210317143529.615584-1-vsementsov@virtuozzo.com/
> 
> 
> 
> Hi,
> 
> This series seems to have some coding style problems. See output below for
> more information:
> 
> Type: series
> Message-id: 20210317143529.615584-1-vsementsov@virtuozzo.com
> Subject: [PATCH v3 00/36] block: update graph permissions update
> 
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> git rev-parse base > /dev/null || exit 0
> git config --local diff.renamelimit 0
> git config --local diff.renames True
> git config --local diff.algorithm histogram
> ./scripts/checkpatch.pl --mailback base..
> === TEST SCRIPT END ===
> 
> Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
>  From https://github.com/patchew-project/qemu
>   * [new tag]         patchew/20210317143529.615584-1-vsementsov@virtuozzo.com -> patchew/20210317143529.615584-1-vsementsov@virtuozzo.com
> Switched to a new branch 'test'
> ac26c21 block: refactor bdrv_node_check_perm()
> 7171293 block: rename bdrv_replace_child_safe() to bdrv_replace_child()
> 9430fba block: refactor bdrv_child_set_perm_safe() transaction action
> 2386ee1 block: inline bdrv_replace_child()
> 41c9fc1 block: inline bdrv_check_perm_common()
> 0d06ade block: drop unused permission update functions
> 72e8fe6 block: bdrv_reopen_multiple: refresh permissions on updated graph
> b7f3a63 block: bdrv_reopen_multiple(): move bdrv_flush to separate pre-prepare
> b9278b8 block: add bdrv_set_backing_noperm() transaction action
> 44f6e69 block: make bdrv_refresh_limits() to be a transaction action
> a671d58 block: make bdrv_unset_inherits_from to be a transaction action
> aa9e316 block: drop ignore_children for permission update functions
> 6e17bb7 block/backup-top: drop .active
> 7292e37 block: introduce bdrv_drop_filter()
> 9e7ce4f block: add bdrv_remove_filter_or_cow transaction action
> a5f3a81 block: adapt bdrv_append() for inserting filters
> ba82bea block: split out bdrv_replace_node_noperm()
> 4c97817 block: add bdrv_attach_child_noperm() transaction action
> 4a4e038 block: add bdrv_attach_child_common() transaction action
> a864516 block: fix bdrv_replace_node_common
> ce2b5ee block: add bdrv_replace_child_safe() transaction action
> 0b5ce80 block: add bdrv_list_* permission update functions
> 5380bbb block: add bdrv_drv_set_perm transaction action
> 965e80a block: use topological sort for permission update
> 35d94b7 block: inline bdrv_child_*() permission functions calls
> 84f3087 block: rewrite bdrv_child_try_set_perm() using bdrv_refresh_perms()
> 9eb28b7 block: refactor bdrv_child* permission functions
> 0c068cb block: bdrv_refresh_perms: check for parents permissions conflict
> 23d55e7 util: add transactions.c
> 68189c0 block: make bdrv_reopen_{prepare, commit, abort} private
> 5780b80 block: drop ctx argument from bdrv_root_attach_child
> 34f4c11 block: BdrvChildClass: add .get_parent_aio_context handler
> e79d608 block: bdrv_append(): don't consume reference
> 87c292c tests/test-bdrv-graph-mod: add test_append_greedy_filter
> 381fa0c tests/test-bdrv-graph-mod: add test_parallel_perm_update
> 6df2206 tests/test-bdrv-graph-mod: add test_parallel_exclusive_write
> 
> === OUTPUT BEGIN ===
> 1/36 Checking commit 6df2206946ee (tests/test-bdrv-graph-mod: add test_parallel_exclusive_write)
> 2/36 Checking commit 381fa0c5db90 (tests/test-bdrv-graph-mod: add test_parallel_perm_update)
> 3/36 Checking commit 87c292ca6696 (tests/test-bdrv-graph-mod: add test_append_greedy_filter)
> 4/36 Checking commit e79d608e8fad (block: bdrv_append(): don't consume reference)
> 5/36 Checking commit 34f4c11ddd7e (block: BdrvChildClass: add .get_parent_aio_context handler)
> 6/36 Checking commit 5780b805277e (block: drop ctx argument from bdrv_root_attach_child)
> 7/36 Checking commit 68189c099a3a (block: make bdrv_reopen_{prepare, commit, abort} private)
> ERROR: Author email address is mangled by the mailing list
> #2:
> Author: Vladimir Sementsov-Ogievskiy via <qemu-devel@nongnu.org>
> 

Who know what is it? Commit message, subject and "From:" header are clean in the email..


-- 
Best regards,
Vladimir

On 3/17/21 10:38 AM, Vladimir Sementsov-Ogievskiy wrote:

>> 6/36 Checking commit 5780b805277e (block: drop ctx argument from
>> bdrv_root_attach_child)
>> 7/36 Checking commit 68189c099a3a (block: make bdrv_reopen_{prepare,
>> commit, abort} private)
>> ERROR: Author email address is mangled by the mailing list
>> #2:
>> Author: Vladimir Sementsov-Ogievskiy via <qemu-devel@nongnu.org>
>>
> 
> Who know what is it? Commit message, subject and "From:" header are
> clean in the email..

The list mangles mails for setups where DKIM/SCP setups are strict
enough that the mail would be rejected by various recipients otherwise.
But I have no idea why the mailing list rewrote the headers for that one
mail, but not the rest of your series - usually, DKIM setups are
persistent enough that it will be an all-or-none conversion to the
entire series.

At any rate, a maintainer can manually fix it for one patch, or you can
resend (if the mailing list keeps mangling headers, you can add a 'From:
' line in the body of your email that will override the mangled header;
but since the list doesn't usually mangle your headers, you may not need
to resort to that).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

On 3/17/21 12:33 PM, Eric Blake wrote:
> On 3/17/21 10:38 AM, Vladimir Sementsov-Ogievskiy wrote:
> 
>>> 6/36 Checking commit 5780b805277e (block: drop ctx argument from
>>> bdrv_root_attach_child)
>>> 7/36 Checking commit 68189c099a3a (block: make bdrv_reopen_{prepare,
>>> commit, abort} private)
>>> ERROR: Author email address is mangled by the mailing list
>>> #2:
>>> Author: Vladimir Sementsov-Ogievskiy via <qemu-devel@nongnu.org>
>>>
>>
>> Who know what is it? Commit message, subject and "From:" header are
>> clean in the email..
> 
> The list mangles mails for setups where DKIM/SCP setups are strict

Sorry, acronym soup got me confused.  I meant DKIM/ SPF, and DMARC.

> enough that the mail would be rejected by various recipients otherwise.
> But I have no idea why the mailing list rewrote the headers for that one
> mail, but not the rest of your series - usually, DKIM setups are
> persistent enough that it will be an all-or-none conversion to the
> entire series.
> 
> At any rate, a maintainer can manually fix it for one patch, or you can
> resend (if the mailing list keeps mangling headers, you can add a 'From:
> ' line in the body of your email that will override the mangled header;
> but since the list doesn't usually mangle your headers, you may not need
> to resort to that).
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

17.03.2021 20:33, Eric Blake wrote:
> On 3/17/21 10:38 AM, Vladimir Sementsov-Ogievskiy wrote:
> 
>>> 6/36 Checking commit 5780b805277e (block: drop ctx argument from
>>> bdrv_root_attach_child)
>>> 7/36 Checking commit 68189c099a3a (block: make bdrv_reopen_{prepare,
>>> commit, abort} private)
>>> ERROR: Author email address is mangled by the mailing list
>>> #2:
>>> Author: Vladimir Sementsov-Ogievskiy via <qemu-devel@nongnu.org>
>>>
>>
>> Who know what is it? Commit message, subject and "From:" header are
>> clean in the email..
> 
> The list mangles mails for setups where DKIM/SCP setups are strict
> enough that the mail would be rejected by various recipients otherwise.
> But I have no idea why the mailing list rewrote the headers for that one
> mail, but not the rest of your series - usually, DKIM setups are
> persistent enough that it will be an all-or-none conversion to the
> entire series.
> 
> At any rate, a maintainer can manually fix it for one patch, or you can
> resend (if the mailing list keeps mangling headers, you can add a 'From:
> ' line in the body of your email that will override the mangled header;
> but since the list doesn't usually mangle your headers, you may not need
> to resort to that).
> 

I'm looking at 7/36 in my mailbox, and don't see where is it mangled?

Looking at message source I see clean "From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>", and "qemu-devel@nongnu.org" is only in Cc:.

And I've sent the series by git-email, so emails in thunderbird came from the mailing list in a clean way.

As well, message looks good in the archive: https://lists.gnu.org/archive/html/qemu-devel/2021-03/msg06288.html

-- 
Best regards,
Vladimir

Am 18.03.2021 um 09:04 hat Vladimir Sementsov-Ogievskiy geschrieben:
> 17.03.2021 20:33, Eric Blake wrote:
> > On 3/17/21 10:38 AM, Vladimir Sementsov-Ogievskiy wrote:
> > 
> > > > 6/36 Checking commit 5780b805277e (block: drop ctx argument from
> > > > bdrv_root_attach_child)
> > > > 7/36 Checking commit 68189c099a3a (block: make bdrv_reopen_{prepare,
> > > > commit, abort} private)
> > > > ERROR: Author email address is mangled by the mailing list
> > > > #2:
> > > > Author: Vladimir Sementsov-Ogievskiy via <qemu-devel@nongnu.org>
> > > > 
> > > 
> > > Who know what is it? Commit message, subject and "From:" header are
> > > clean in the email..
> > 
> > The list mangles mails for setups where DKIM/SCP setups are strict
> > enough that the mail would be rejected by various recipients otherwise.
> > But I have no idea why the mailing list rewrote the headers for that one
> > mail, but not the rest of your series - usually, DKIM setups are
> > persistent enough that it will be an all-or-none conversion to the
> > entire series.
> > 
> > At any rate, a maintainer can manually fix it for one patch, or you can
> > resend (if the mailing list keeps mangling headers, you can add a 'From:
> > ' line in the body of your email that will override the mangled header;
> > but since the list doesn't usually mangle your headers, you may not need
> > to resort to that).
> > 
> 
> I'm looking at 7/36 in my mailbox, and don't see where is it mangled?

My primary copy is good, too, but that seems to be because you CCed me
directly, so it didn't even go through the list. My copy from qemu-devel
seems to be mangled, though. You can look at Patchew's here:

https://patchew.org/QEMU/20210317143529.615584-8-vsementsov@virtuozzo.com/mbox

But that you CCed me means it's not a practical problem for this series.

Kevin

Am 17.03.2021 um 15:34 hat Vladimir Sementsov-Ogievskiy geschrieben:
> Hi all!
> 
> Finally, I finished v3. Phew.
> 
> Missed a soft-freeze. Should we consider it a bugfix? There are bugfixes
> here but they are mostly theoretical. So, up to Kevin, should it go to
> current release or to the next..
> 
> The main point of the series is fixing some permission update problems
> (see patches 01-03 as examples), that in turn makes possible more clean
> inserting and removing of filters (see patch 26 where .active field is
> dropped finally from backup-top filter, we don't need a workaround
> anymore).
> 
> The series brings util/transactions.c (patch 10) and use of it in
> block.c, which allows clean block graph change transactions, with
> possibility of reverting all modifications (movement and removement of
> children, changing aio context, changing permissions) in reverse order
> on failure path.
> 
> The series also helps Alberto's "Allow changing bs->file on reopen"
> which we want to merge prior dropping x- prefix from blockdev-reopen
> command.

Ok, I made it through the whole series. Looking quite good, I just had
some minor comments, but maybe not quite trival enough to just fix them
up while applying.

I had comments on patches 1, 4, 8, 18 and 22. With these addressed, you
can add to all patches in the whole series:
Reviewed-by: Kevin Wolf <kwolf@redhat.com>

v4 should then be the final version. :-)

Kevin

Am 17.03.2021 um 15:34 hat Vladimir Sementsov-Ogievskiy geschrieben:
> Hi all!
> 
> Finally, I finished v3. Phew.
> 
> Missed a soft-freeze. Should we consider it a bugfix? There are bugfixes
> here but they are mostly theoretical. So, up to Kevin, should it go to
> current release or to the next..
> 
> The main point of the series is fixing some permission update problems
> (see patches 01-03 as examples), that in turn makes possible more clean
> inserting and removing of filters (see patch 26 where .active field is
> dropped finally from backup-top filter, we don't need a workaround
> anymore).
> 
> The series brings util/transactions.c (patch 10) and use of it in
> block.c, which allows clean block graph change transactions, with
> possibility of reverting all modifications (movement and removement of
> children, changing aio context, changing permissions) in reverse order
> on failure path.
> 
> The series also helps Alberto's "Allow changing bs->file on reopen"
> which we want to merge prior dropping x- prefix from blockdev-reopen
> command.

With the minor comments I had so far fixed, patches 1-13 are:
Reviewed-by: Kevin Wolf <kwolf@redhat.com>

I'll continue next week.

Kevin