Am 16.11.2018 um 17:45 hat Max Reitz geschrieben: > These are fixes for issues I found when looking after something Berto > has reported. The second patch fixes that issue Berto found, the first > one is only kind of related. > > For the first patch: bdrv_reopen_abort() or bdrv_reopen_commit() are > called only if the respective bdrv_reopen_prepare() has succeeded. > However, bdrv_reopen_prepare() can fail even if the block driver’s > .bdrv_reopen_prepare() succeeded. In that case, the block driver is > expecting a .bdrv_reopen_abort() or .bdrv_reopen_commit(), but will > never get it. > > This is fixed by making bdrv_reopen_prepare() call .bdrv_reopen_abort() > if an error occurs after .bdrv_reopen_prepare() has already succeeded. > > In practice this just prevents resource leaks, so nothing too dramatic > (fine for qemu-next). It also means I’ve been incapable of writing a > test, sorry. > > > The second issue is more severe: file-posix applies the inverse share > locks when reopening. Now the user can only directly do reopens from > the monitor for now, so that wouldn’t be so bad. But of course there > are other places in qemu that implicitly reopen nodes, like dropping > R/W to RO or gaining R/W from RO. Most of these places are symmetrical > at least (they’ll get R/W on an RO image for a short period of time and > then drop back to RO), so you’ll see the wrong shared permission locks > only for a short duration. But at least blockdev-snapshot and > blockdev-snapshot-sync are one way; they drop R/W to RO and stay there. > So if you do a blockdev-snapshot*, the shared permission bits will be > flipped. This is therefore very much user-visible. > > It’s not catastrophic, though, so I’m not sure whether we need it in > 3.1. It’s definitely a bugfix, and I think we have patches queued for > the next RC already, so I think it makes sense to include at least > patches 2 and 3 as well.
Thanks, applied to the block branch. Kevin