Message ID | 20201127144522.29991-1-vsementsov@virtuozzo.com |
---|---|
Headers | show |
Series | block: update graph permissions update | expand |
On Fri 27 Nov 2020 03:44:54 PM CET, Vladimir Sementsov-Ogievskiy via wrote: > These functions are called only from bdrv_reopen_multiple() in block.c. > No reason to publish them. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Berto
ping 27.11.2020 17:44, Vladimir Sementsov-Ogievskiy wrote: > Hi all! > > Here is a proposal of updating graph changing procedures. > > The thing brought me here is a question about "activating" filters after > insertion, which is done in mirror_top and backup_top. The problem is > that we can't simply avoid permission conflict when inserting the > filter: during insertion old permissions of relations to be removed > conflicting with new permissions of new created relations. And current > solution is supporting additional "inactive" mode for the filter when it > doesn't require any permissions. > > I suggest to change the order of operations: let's first do all graph > relations modifications and then refresh permissions. Of course we'll > need a way to restore old graph if refresh fails. > > Another problem with permission update is that we update permissions in > order of DFS which is not always correct. Better is update node when all > its parents already updated and require correct permissions. This needs > a topological sort of nodes prior to permission update, see more in > patches later. > > Patches plan: > > 01,02 - add failing tests to illustrate conceptual problems of current > permission update system. > [Here is side suggestion: we usually add tests after fix, so careful > reviewer has to change order of patches to check that test fails before > fix. I add tests in the way the may be simply executed but not yet take > part in make check. It seems more native: first show the problem, then > fix it. And when fixed, make tests available for make check] > > 03-09 - some perparations, refactorings which may go in separate > > 10 - new transaction API > > 15 - toplogical sort implemented for permission update, one of new tests > now pass > > 19 - improve bdrv_replace_node. second new test now pass > > 26 - drop .active field and activation procedure for backup-top! > > 30 - update bdrv_reopen_multiple. At this point everything is using new > paradigm of permission update > > 31-36 - post refactoring, drop old interfaces, we are done. > > Note, that this series does nothing with another graph-update problem > discussed under "[PATCH RFC 0/5] Fix accidental crash in iotest 30". > > The series based on block-next Max's branch and can be found here: > > git: https://src.openvz.org/scm/~vsementsov/qemu.git > tag: up-block-topologic-perm-v2 > > Vladimir Sementsov-Ogievskiy (36): > tests/test-bdrv-graph-mod: add test_parallel_exclusive_write > tests/test-bdrv-graph-mod: add test_parallel_perm_update > block: bdrv_append(): don't consume reference > block: bdrv_append(): return status > block: add bdrv_parent_try_set_aio_context > 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 > block: return value from bdrv_replace_node() > util: add transactions.c > block: bdrv_refresh_perms: check parents compliance > 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_backing transaction action > block: introduce bdrv_drop_filter() > block/backup-top: drop .active > block: drop ignore_children for permission update functions > block: add bdrv_set_backing_noperm() transaction action > blockdev: qmp_x_blockdev_reopen: acquire all contexts > 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 | 20 +- > include/block/block_int.h | 8 +- > include/qemu/transactions.h | 46 ++ > block.c | 1319 ++++++++++++++++++++--------------- > block/backup-top.c | 39 +- > block/block-backend.c | 13 +- > block/commit.c | 7 +- > block/file-posix.c | 84 +-- > block/mirror.c | 9 +- > blockdev.c | 33 +- > blockjob.c | 11 +- > tests/test-bdrv-drain.c | 2 +- > tests/test-bdrv-graph-mod.c | 122 +++- > util/transactions.c | 81 +++ > tests/qemu-iotests/283.out | 2 +- > util/meson.build | 1 + > 16 files changed, 1100 insertions(+), 697 deletions(-) > create mode 100644 include/qemu/transactions.h > create mode 100644 util/transactions.c >
Am 27.11.2020 um 15:44 hat Vladimir Sementsov-Ogievskiy geschrieben: > These functions are called only from bdrv_reopen_multiple() in block.c. > No reason to publish them. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>