Re: [RFC PATCH 0/2] Allow changing bs->file on reopen - Vladimir Sementsov-Ogievskiy

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
To: Alberto Garcia <berto@igalia.com>, qemu-devel@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>,
	Kashyap Chamarthy <kchamart@redhat.com>,
	qemu-block@nongnu.org, Max Reitz <mreitz@redhat.com>
Subject: Re: [RFC PATCH 0/2] Allow changing bs->file on reopen
Date: Mon, 18 Jan 2021 13:22:49 +0300	[thread overview]
Message-ID: <dafbbcb3-9d87-ab04-5413-eac551cdfe44@virtuozzo.com> (raw)
In-Reply-To: <cover.1610715661.git.berto@igalia.com>

15.01.2021 16:02, Alberto Garcia wrote:
> Hi,
> 
> during the past months we talked about making x-blockdev-reopen stable
> API, and one of the missing things was having support for changing
> bs->file. See here for the discusssion (I can't find the message from
> Kashyap that started the thread in the web archives):
> 
>     https://lists.gnu.org/archive/html/qemu-block/2020-10/msg00922.html
> 
> I was testing this and one of the problems that I found was that
> removing a filter node using this command is tricky because of the
> permission system, see here for details:
> 
>     https://lists.gnu.org/archive/html/qemu-block/2020-12/msg00092.html
> 
> The good news is that Vladimir posted a set of patches that changes
> the way that permissions are updated on reopen:
> 
>     https://lists.gnu.org/archive/html/qemu-block/2020-11/msg00745.html
> 
> I was testing if this would be useful to solve the problem that I
> mentioned earlier and it seems to be the case so I wrote a patch to
> add support for changing bs->file, along with a couple of test cases.
> 
> This is still an RFC but you can see the idea.

Good idea and I glad to see that my patches help:)

Hmm, still, removing a filter which want to unshare WRITE even when doesn't have any parents will be a problem anyway, so we'll need a new command to drop filter with a logic like in bdrv_drop_filter in my series.

Or, we can introduce multiple reopen.. So that x-blockdev-reopen will take a list of BlockdevOptions, and do all modifications in one transaction. Than we'll be able to drop filter by transactional update of top node child and removing filter child link.

> 
> These patches apply on top of Vladimir's branch:
> 
> git: https://src.openvz.org/scm/~vsementsov/qemu.git
> tag: up-block-topologic-perm-v2
> 
> Opinions are very welcome!
> 
> Berto
> 
> Alberto Garcia (2):
>    block: Allow changing bs->file on reopen
>    iotests: Update 245 to support replacing files with x-blockdev-reopen
> 
>   include/block/block.h      |  1 +
>   block.c                    | 61 ++++++++++++++++++++++++++++++++++++++
>   tests/qemu-iotests/245     | 61 +++++++++++++++++++++++++++++++++++---
>   tests/qemu-iotests/245.out |  4 +--
>   4 files changed, 121 insertions(+), 6 deletions(-)
> 

-- 
Best regards,
Vladimir