Re: x-blockdev-reopen & block-dirty-bitmaps

[Peter Krempa]

On Mon, Feb 17, 2020 at 10:52:31 +0100, Kevin Wolf wrote:
> Am 14.02.2020 um 21:32 hat John Snow geschrieben:
> > On 2/14/20 3:19 PM, Kevin Wolf wrote:
> > > Am 14.02.2020 um 19:54 hat John Snow geschrieben:
> > >> Hi, what work remains to make this a stable interface, is it known?
> > >>
> > >> We're having a problem with bitmaps where in some cases libvirt wants to
> > >> commit an image back down to a base image but -- for various reasons --
> > >> the bitmap was left enabled in the backing image, so it would accrue new
> > >> writes during the commit.
> > >>
> > >> Normally, when creating a snapshot using blockdev-snapshot, the backing
> > >> file becomes RO and all of the bitmaps become RO too.
> > >>
> > >> The goal is to be able to disable (or enable) bitmaps from a backing
> > >> file before (or atomically just before) a commit operation to allow
> > >> libvirt greater control on snapshot commit.
> > >>
> > >> Now, in my own testing, we can reopen a backing file just fine, delete
> > >> or disable a bitmap and be done with it -- but the interface isn't
> > >> stable, so libvirt will be reluctant to use such tricks.

Well, while we probably want it to be stable for upstream acceptance
that didn't prevent me from actually trying to use reopening. It would
be probably frowned upon if I tried to use it upstream.

The problem is that we'd have to carry the compatibility code for at
least the two possible names of the command if nothing else changes and
also the fact that once the command is declared stable, some older
libvirt versions might not know to use it.

The implementation was surprisingly easy though and works well to reopen
the backing files in RW mode. The caveat was that the reopen somehow
still didn't reopen the bitmaps and qemu ended up reporting:

libvirt-1-format: Failed to make dirty bitmaps writable: Cannot update bitmap 
directory: Bad file descriptor

So unfortunately it didn't work out for that scenario.

<sidetrack alert>

Also I'm afraid I have another use case for it:

oVirt when doing their 'live storage migration' actually uses libvirt to
mirror only the top layer in shallow mode and copies everything else
while the mirror is running using qemu-img.

Prior to libvirt's use of -blockdev this worked well, because qemu
reopened the mirror destination (which caused to open the backing files)
only at the end. With -blockdev we have to open the backing files right
away so that they can be properly installed as backing of the image
being mirrored and oVirt's qemu-img instance gets a locking error as the
images are actually opened for reading already.

I'm afraid that we won't be able to restore the previous semantics
without actually opening the backing files after the copy is
synchronized before completing the job and then installing them as the
backing via blockdev-reopen.

Libvirt's documentation was partially alibistic [1] and asked the user to
actually provide a working image but oVirt actually exploited the qemu
behaviour to allow folding the two operations together.

[1] https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainBlockCopy

> > >>
> > >> Probably a loaded question, but:
> > >>
> > >> - What's needed to make the interface stable?
> > >> - Are there known problem points?
> > >> - Any suggestions for workarounds in the meantime?
> > > 
> > > I think I've asked this before, but I don't remember the answer...
> > > 
> > > What would be the problem with letting the enable/disable command
> > > temporarily reopen the backing file read-write, like the commit job [1]
> > > does?
> > > 
> > 
> > I guess no problem? I wouldn't want it to do this automatically, but
> > perhaps we could add a "force=True" bool where it tries to do just this.
> > 
> > (And once reopen works properly we could deprecate this workaround again.)
> 
> I'm not sure if I would view this only as a workaround, but if you like
> it better with force=true, I won't object either.

I'm wondering whether adding a feature deprecating it soon is even worth
doing. From libvirt's point of view it would be comparable to using the
x-prefixed command, with just slightly longer transition period. (
deleting the x-prefix is a very hard transition to cope with)