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.
v2:
- Patch 1: Reuse the existing error path instead of creating a new one
[Berto]
git-backport-diff against v1:
Key:
[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively
001/3:[0025] [FC] 'block: Always abort reopen after prepare succeeded'
002/3:[----] [--] 'file-posix: Fix shared locks on reopen commit'
003/3:[----] [--] 'iotests: Test file-posix locking and reopen'
Max Reitz (3):
block: Always abort reopen after prepare succeeded
file-posix: Fix shared locks on reopen commit
iotests: Test file-posix locking and reopen
block.c | 12 +++++++
block/file-posix.c | 2 +-
tests/qemu-iotests/182 | 71 ++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/182.out | 9 +++++
4 files changed, 93 insertions(+), 1 deletion(-)
--
2.17.2
