| Message ID | cover.1610715661.git.berto@igalia.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Allow changing bs->file on reopen | expand |
On Fri, Jan 15, 2021 at 02:02:36PM +0100, Alberto Garcia wrote: > Hi, 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 Yeah, I noticed that too -- seems like it got "lost" somehow :-(. For the record, I've attached here the original e-mail I sent on 06-OCT-2020 that started the above thread. Thanks for working on this! > 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. > > 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(-) > > -- > 2.20.1 >
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(-) >
On Mon 18 Jan 2021 11:22:49 AM CET, Vladimir Sementsov-Ogievskiy wrote: >> 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. Do you have an example? Berto
20.01.2021 16:51, Alberto Garcia wrote: > On Mon 18 Jan 2021 11:22:49 AM CET, Vladimir Sementsov-Ogievskiy wrote: >>> 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. > > Do you have an example? > backup_top filter
Am 18.01.2021 um 11:22 hat Vladimir Sementsov-Ogievskiy geschrieben: > 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. Internally, we already have reopen queues anyway, so it would make sense to me to expose them externally and take a list of BlockdevOptions. This way we should be able to do even complex changes of the graph where adding some edges requires the removal of other edges in a single atomic operation. Kevin
On Thu 21 Jan 2021 11:52:17 AM CET, Kevin Wolf wrote: >> 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. > > Internally, we already have reopen queues anyway, so it would make > sense to me to expose them externally and take a list of > BlockdevOptions. This way we should be able to do even complex > changes of the graph where adding some edges requires the removal of > other edges in a single atomic operation. So you mean changing the signature to something like this? { 'command': 'x-blockdev-reopen', 'data': { 'options': ['BlockdevOptions'] } } It should be easy to make that change, I can have a look. Berto
Am 05.02.2021 um 13:47 hat Alberto Garcia geschrieben: > On Thu 21 Jan 2021 11:52:17 AM CET, Kevin Wolf wrote: > >> 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. > > > > Internally, we already have reopen queues anyway, so it would make > > sense to me to expose them externally and take a list of > > BlockdevOptions. This way we should be able to do even complex > > changes of the graph where adding some edges requires the removal of > > other edges in a single atomic operation. > > So you mean changing the signature to something like this? > > { 'command': 'x-blockdev-reopen', > 'data': { 'options': ['BlockdevOptions'] } } > > It should be easy to make that change, I can have a look. Yes, this is what I had in mind. Kevin