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

[Kevin Wolf]

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