Hi all!

And here is v4.

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

01: drop misleading comment
04: add missed bdrv_unref to new bdrv_append() caller
08: s/Than/Then/ in comment 
18: add missed ".clean = g_free" to bdrv_attach_child_common_drv
    fix typo in comment
    qsd-jobs test output updated 
22: rewrite comment. I've changed your last wording, be free to return
    it back if you like it more. 
    
05,06,09: Add Alberto's r-b

all patches except for 18, for new test output: Add Kevin's r-b

===

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.

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                               | 1329 +++++++++++++++----------
 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      |  209 +++-
 util/transactions.c                   |   96 ++
 MAINTAINERS                           |    6 +
 tests/qemu-iotests/245                |    2 +-
 tests/qemu-iotests/283.out            |    2 +-
 tests/qemu-iotests/tests/qsd-jobs.out |    2 +-
 util/meson.build                      |    1 +
 20 files changed, 1261 insertions(+), 674 deletions(-)
 create mode 100644 include/qemu/transactions.h
 create mode 100644 util/transactions.c

-- 
2.29.2

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



Hi,

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

Type: series
Message-id: 20210428151804.439460-1-vsementsov@virtuozzo.com
Subject: [PATCH v4 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
 - [tag update]      patchew/20210428141655.387430-1-f4bug@amsat.org -> patchew/20210428141655.387430-1-f4bug@amsat.org
 * [new tag]         patchew/20210428151804.439460-1-vsementsov@virtuozzo.com -> patchew/20210428151804.439460-1-vsementsov@virtuozzo.com
Switched to a new branch 'test'
a0ef51a block: refactor bdrv_node_check_perm()
7429fe1 block: rename bdrv_replace_child_safe() to bdrv_replace_child()
370a34f block: refactor bdrv_child_set_perm_safe() transaction action
d1a11c5 block: inline bdrv_replace_child()
ab4725c block: inline bdrv_check_perm_common()
1e87dc2 block: drop unused permission update functions
7f7c03c block: bdrv_reopen_multiple: refresh permissions on updated graph
dae8ccf block: bdrv_reopen_multiple(): move bdrv_flush to separate pre-prepare
3882433 block: add bdrv_set_backing_noperm() transaction action
4dbfab2 block: make bdrv_refresh_limits() to be a transaction action
0cf05c0 block: make bdrv_unset_inherits_from to be a transaction action
1268c27 block: drop ignore_children for permission update functions
3b2ce34 block/backup-top: drop .active
f8d8895 block: introduce bdrv_drop_filter()
5bf1218 block: add bdrv_remove_filter_or_cow transaction action
00b761a block: adapt bdrv_append() for inserting filters
dd27bfe block: split out bdrv_replace_node_noperm()
2b2d8e4 block: add bdrv_attach_child_noperm() transaction action
3c44761 block: add bdrv_attach_child_common() transaction action
3f07f35 block: fix bdrv_replace_node_common
e2a0ea6 block: add bdrv_replace_child_safe() transaction action
92f4628 block: add bdrv_list_* permission update functions
84a955c block: add bdrv_drv_set_perm transaction action
769127d block: use topological sort for permission update
1a3293c block: inline bdrv_child_*() permission functions calls
0eafad8 block: rewrite bdrv_child_try_set_perm() using bdrv_refresh_perms()
02b7d94 block: refactor bdrv_child* permission functions
081d242 block: bdrv_refresh_perms: check for parents permissions conflict
e7d5541 util: add transactions.c
a5835ab block: make bdrv_reopen_{prepare, commit, abort} private
eebe328 block: drop ctx argument from bdrv_root_attach_child
6bfb813 block: BdrvChildClass: add .get_parent_aio_context handler
faf9f5a block: bdrv_append(): don't consume reference
09a714e tests/test-bdrv-graph-mod: add test_append_greedy_filter
a2e5008 tests/test-bdrv-graph-mod: add test_parallel_perm_update
7e9a7a4 tests/test-bdrv-graph-mod: add test_parallel_exclusive_write

=== OUTPUT BEGIN ===
1/36 Checking commit 7e9a7a4d740a (tests/test-bdrv-graph-mod: add test_parallel_exclusive_write)
2/36 Checking commit a2e500832942 (tests/test-bdrv-graph-mod: add test_parallel_perm_update)
3/36 Checking commit 09a714e2f77d (tests/test-bdrv-graph-mod: add test_append_greedy_filter)
4/36 Checking commit faf9f5a35e07 (block: bdrv_append(): don't consume reference)
5/36 Checking commit 6bfb813f671d (block: BdrvChildClass: add .get_parent_aio_context handler)
6/36 Checking commit eebe328485c7 (block: drop ctx argument from bdrv_root_attach_child)
7/36 Checking commit a5835ab099b6 (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 e7d55412e1a6 (util: add transactions.c)
9/36 Checking commit 081d242cc6c7 (block: bdrv_refresh_perms: check for parents permissions conflict)
10/36 Checking commit 02b7d94ba496 (block: refactor bdrv_child* permission functions)
11/36 Checking commit 0eafad8a0bdb (block: rewrite bdrv_child_try_set_perm() using bdrv_refresh_perms())
12/36 Checking commit 1a3293c8af87 (block: inline bdrv_child_*() permission functions calls)
13/36 Checking commit 769127da032f (block: use topological sort for permission update)
14/36 Checking commit 84a955ce7cdd (block: add bdrv_drv_set_perm transaction action)
15/36 Checking commit 92f46287bd3e (block: add bdrv_list_* permission update functions)
16/36 Checking commit e2a0ea66e2c7 (block: add bdrv_replace_child_safe() transaction action)
17/36 Checking commit 3f07f3545be4 (block: fix bdrv_replace_node_common)
18/36 Checking commit 3c44761b6973 (block: add bdrv_attach_child_common() transaction action)
19/36 Checking commit 2b2d8e426b90 (block: add bdrv_attach_child_noperm() transaction action)
20/36 Checking commit dd27bfe67248 (block: split out bdrv_replace_node_noperm())
21/36 Checking commit 00b761a9f56a (block: adapt bdrv_append() for inserting filters)
22/36 Checking commit 5bf12184a1a5 (block: add bdrv_remove_filter_or_cow transaction action)
23/36 Checking commit f8d88950f1b4 (block: introduce bdrv_drop_filter())
24/36 Checking commit 3b2ce348f777 (block/backup-top: drop .active)
25/36 Checking commit 1268c27f3f53 (block: drop ignore_children for permission update functions)
26/36 Checking commit 0cf05c0e2aef (block: make bdrv_unset_inherits_from to be a transaction action)
27/36 Checking commit 4dbfab28ca96 (block: make bdrv_refresh_limits() to be a transaction action)
28/36 Checking commit 3882433ee254 (block: add bdrv_set_backing_noperm() transaction action)
29/36 Checking commit dae8ccf5ba24 (block: bdrv_reopen_multiple(): move bdrv_flush to separate pre-prepare)
30/36 Checking commit 7f7c03cdd8d4 (block: bdrv_reopen_multiple: refresh permissions on updated graph)
31/36 Checking commit 1e87dc2426e3 (block: drop unused permission update functions)
32/36 Checking commit ab4725cc446f (block: inline bdrv_check_perm_common())
33/36 Checking commit d1a11c5c0b9a (block: inline bdrv_replace_child())
34/36 Checking commit 370a34fc4980 (block: refactor bdrv_child_set_perm_safe() transaction action)
35/36 Checking commit 7429fe119c1c (block: rename bdrv_replace_child_safe() to bdrv_replace_child())
36/36 Checking commit a0ef51a64512 (block: refactor bdrv_node_check_perm())
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20210428151804.439460-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

Am 28.04.2021 um 17:17 hat Vladimir Sementsov-Ogievskiy geschrieben:
> Hi all!
> 
> And here is v4.

Thanks, applied to the block branch.

Though the error message shown by the test in patch 18 does need some
improvement on top. We should probably name both conflicting users and
who blocks whom in a way that is neutral as to which user is the new
one.

Also, curious that again patch 7 (and only patch 7) got From: mangled by
the mailing list. Seems there is something in it that Mailman doesn't
like?

Kevin

28.04.2021 20:03, Kevin Wolf wrote:
> Am 28.04.2021 um 17:17 hat Vladimir Sementsov-Ogievskiy geschrieben:
>> Hi all!
>>
>> And here is v4.
> 
> Thanks, applied to the block branch.

Thanks! And thanks a lot for reviewing!

> 
> Though the error message shown by the test in patch 18 does need some
> improvement on top. We should probably name both conflicting users and
> who blocks whom in a way that is neutral as to which user is the new
> one.

I'll look at it next week.

> 
> Also, curious that again patch 7 (and only patch 7) got From: mangled by
> the mailing list. Seems there is something in it that Mailman doesn't
> like?
> 

Very interesting.. Curly braces in subject maybe? But I think, that not a first time I use them..


-- 
Best regards,
Vladimir

28.04.2021 22:50, Vladimir Sementsov-Ogievskiy wrote:
> 28.04.2021 20:03, Kevin Wolf wrote:
>> Am 28.04.2021 um 17:17 hat Vladimir Sementsov-Ogievskiy geschrieben:
>>> Hi all!
>>>
>>> And here is v4.
>>
>> Thanks, applied to the block branch.
> 
> Thanks! And thanks a lot for reviewing!
> 
>>
>> Though the error message shown by the test in patch 18 does need some
>> improvement on top. We should probably name both conflicting users and
>> who blocks whom in a way that is neutral as to which user is the new
>> one.
> 
> I'll look at it next week.
> 
>>
>> Also, curious that again patch 7 (and only patch 7) got From: mangled by
>> the mailing list. Seems there is something in it that Mailman doesn't
>> like?
>>
> 
> Very interesting.. Curly braces in subject maybe? But I think, that not a first time I use them..
> 
> 

And now I think that is the reason.

Look, in my 64bit block-layer series, first email is mangled, and it has curly braces in subject too:

  https://patchew.org/QEMU/20210324205132.464899-1-vsementsov@virtuozzo.com/


I'll resend 64bit series soon with s/{}/()/ and we'll see if it help.

-- 
Best regards,
Vladimir