[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] block: drop BLK_PERM_GRAPH_MOD
From: |
Eric Blake |
Subject: |
Re: [PATCH] block: drop BLK_PERM_GRAPH_MOD |
Date: |
Tue, 20 Jul 2021 15:49:59 -0500 |
User-agent: |
NeoMutt/20210205-619-921509 |
On Tue, Jul 20, 2021 at 05:22:29PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> First, this permission never protected node from being changed, as
a node
> 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 it child: read, write,
its child
> resize. Graph modification operations are something completely
> different.
>
> The only place, where BLK_PERM_GRAPH_MOD is used as "perm" (not shared
s/place,/place/
> 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 corresponding byte in
locking the
> file-posix doesn't make sense at all.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> +++ b/include/block/block.h
> @@ -269,12 +269,13 @@ enum {
> BLK_PERM_RESIZE = 0x08,
>
> /**
> - * This permission is required to change the node that this BdrvChild
> - * points to.
> + * There was a removed now BLK_PERM_GRAPH_MOD, with value of 0x10. QEMU
> 6.1
There was a now-removed bit BLK_PERM_GRAPH_MOD,
> + * and earlier still my lock corresponding byte in block/file-posix
> locking.
s/still my lock/may still lock the/
> + * So, implementing some new permission should be very careful to not
> + * interfere with this old unused thing.
> */
> - BLK_PERM_GRAPH_MOD = 0x10,
>
> - BLK_PERM_ALL = 0x1f,
> + BLK_PERM_ALL = 0x0f,
>
> DEFAULT_PERM_PASSTHROUGH = BLK_PERM_CONSISTENT_READ
> | BLK_PERM_WRITE
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org