[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2] block: drop BLK_PERM_GRAPH_MOD
From: |
Eric Blake |
Subject: |
Re: [PATCH v2] block: drop BLK_PERM_GRAPH_MOD |
Date: |
Fri, 3 Sep 2021 13:47:26 -0500 |
User-agent: |
NeoMutt/20210205-739-420e15 |
On Thu, Sep 02, 2021 at 12:37:54PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> First, this permission never protected a node from being changed, as
> generic child-replacing functions don't check it.
>
> Second, it's a strange thing: it presents a permission of parent node
> to change its child. But generally, children are replaced by different
> mechanisms, like jobs or qmp commands, not by nodes.
>
> Graph-mod permission is hard to understand. All other permissions
> describe operations which done by parent node on its child: read,
> write, resize. Graph modification operations are something completely
> different.
>
> The only place where BLK_PERM_GRAPH_MOD is used as "perm" (not shared
> perm) is mirror_start_job, for s->target. Still modern code should use
> bdrv_freeze_backing_chain() to protect from graph modification, if we
> don't do it somewhere it may be considered as a bug. So, it's a bit
> risky to drop GRAPH_MOD, and analyzing of possible loss of protection
> is hard. But one day we should do it, let's do it now.
>
> One more bit of information is that locking the corresponding byte in
> file-posix doesn't make sense at all.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>
> v2: fix grammar
>
> qapi/block-core.json | 7 ++-----
> include/block/block.h | 9 +++++----
> block.c | 7 +------
> block/commit.c | 1 -
> block/mirror.c | 15 +++------------
> hw/block/block.c | 3 +--
> scripts/render_block_graph.py | 1 -
> tests/qemu-iotests/273.out | 4 ----
> 8 files changed, 12 insertions(+), 35 deletions(-)
>
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 06674c25c9..6fa2c4ab82 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -1825,14 +1825,11 @@
> #
> # @resize: This permission is required to change the size of a block node.
> #
> -# @graph-mod: This permission is required to change the node that this
> -# BdrvChild points to.
> -#
Do we need to mention that graph-mod was removed in 6.2?
> # Since: 4.0
> ##
> { 'enum': 'BlockPermission',
> - 'data': [ 'consistent-read', 'write', 'write-unchanged', 'resize',
> - 'graph-mod' ] }
> + 'data': [ 'consistent-read', 'write', 'write-unchanged', 'resize' ] }
> +
> ##
> # @XDbgBlockGraphEdge:
> #
Otherwise the patch makes sense to me, but I'd rather that Kevin chime in.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org