Am 18.02.2020 um 13:58 hat Peter Krempa geschrieben: > 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.
I think this is exactly the thing we need before we can mark it stable: Some evidence that it actually provides the functionality that management tools need. So thanks for giving it a try. > 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. I'm not completely sure, but this sounds a bit like a reopen bug in the file-posix driver to me, where we keep using the old file descriptor somewhere? Someone (TM) should turn this into a qemu-iotests case and then we can debug it. > <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 This sounds like a case that blockdev-snapshot might be able to solve: After the offline copy has completed, you blockdev-add the whole backing chain for the target and then use blockdev-snapshot to add the active layer (that had 'backing': null) to it. > > > >> > > > >> 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) It would only make sense if we made it work that way as a permanent solution. Kevin
