qemu-block
[Top][All Lists]
Advanced

[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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]