qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-block] [PATCH for-3.1? 0/3] block: Fix two minor reopen issues


From: Max Reitz
Subject: [Qemu-block] [PATCH for-3.1? 0/3] block: Fix two minor reopen issues
Date: Fri, 16 Nov 2018 16:52:59 +0100

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.


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                    | 17 +++++++--
 block/file-posix.c         |  2 +-
 tests/qemu-iotests/182     | 71 ++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/182.out |  9 +++++
 4 files changed, 96 insertions(+), 3 deletions(-)

-- 
2.17.2




reply via email to

[Prev in Thread] Current Thread [Next in Thread]