Re: [RFC PATCH v2 3/4] block: Support multiple reopening with x-blockdev-reopen

[Peter Krempa]

On Mon, Mar 01, 2021 at 12:07:26 +0100, Kevin Wolf wrote:
> Am 25.02.2021 um 18:02 hat Vladimir Sementsov-Ogievskiy geschrieben:
> > 24.02.2021 15:33, Kevin Wolf wrote:
> > > Am 09.02.2021 um 09:03 hat Vladimir Sementsov-Ogievskiy geschrieben:
> > > > 08.02.2021 21:44, Alberto Garcia wrote:
> > > > > Signed-off-by: Alberto Garcia <berto@igalia.com>
> > > > > ---
> > > > >    qapi/block-core.json       |  2 +-
> > > > >    include/block/block.h      |  1 +
> > > > >    block.c                    | 16 +++++--
> > > > >    blockdev.c                 | 85 
> > > > > +++++++++++++++++++++-----------------
> > > > >    tests/qemu-iotests/155     |  9 ++--
> > > > >    tests/qemu-iotests/165     |  4 +-
> > > > >    tests/qemu-iotests/245     | 27 +++++++-----
> > > > >    tests/qemu-iotests/248     |  2 +-
> > > > >    tests/qemu-iotests/248.out |  2 +-
> > > > >    tests/qemu-iotests/298     |  4 +-
> > > > >    10 files changed, 89 insertions(+), 63 deletions(-)
> > > > > 
> > > > > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > > > > index c0e7c23331..b9fcf20a81 100644
> > > > > --- a/qapi/block-core.json
> > > > > +++ b/qapi/block-core.json
> > > > > @@ -4177,7 +4177,7 @@
> > > > >    # Since: 4.0
> > > > >    ##
> > > > >    { 'command': 'x-blockdev-reopen',
> > > > > -  'data': 'BlockdevOptions', 'boxed': true }
> > > > > +  'data': { 'options': ['BlockdevOptions'] } }
> > > > 
> > > > Do we also want to drop x- prefix?
> > > 
> > > libvirt really wants to have a stable blockdev-reopen interface in 6.0
> > > because enabling the incremental backup code depends on this (they just
> > > toggle the readonly flag if I understand correctly, so most of the work
> > > we're currently doing isn't even relevant at this moment for libvirt).
> > 
> > Do you know what is the case exactly? If they do it to remove dirty bitmap
> > from backing image after snapshot operation, probably we'd better improve
> > block-dirty-bitmap-remove command to be able to reopen r-o image?
> > 
> > (I just recently faced such a task)
> 
> I think it was to switch nodes between read-only and read-write, but I
> don't remember the exact context.
> 
> Peter, can you add the details?

Libvirt indeed has code to drive blockdev-reopen to use it to toggle the
read-write state for block dirty bitmap manipulation.

Few of the cases where we use it:

- after blockjobs to toggle the destination image writable (the
  blockjobs turns it RO when it finishes) so that we can merge the
  appropriate bitmaps

- for deletion of bitmaps in backing images in the checkpoint APIs

Both of the operations really require blockdev-reopen and are required
for the incremental backup feature as whole since bitmap manipulation is
crucial for it and it doesn't happen automatically.

The blockdev code has the facilities to also use blockdev-reopen to
change RO/RW state for blockjobs if it will be needed in the future or
it can be later used to even replace the auto-read-only feature of the
protocol node drivers which were actually added to make -blockdev work
in libvirt's s-virt environment properly.