Re: qcow2 api not secured by mutex lock

[Kevin Wolf <kwolf@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1400 bytes --]

Am 19.12.2019 um 11:35 hat Max Reitz geschrieben:
> On 19.12.19 11:02, Kevin Wolf wrote:
> > Am 18.12.2019 um 11:28 hat Vladimir Sementsov-Ogievskiy geschrieben:
> 
> [...]
> 
> >> qcow2_write_snapshots actually called unlocked from
> >> qcow2_check_fix_snapshot_table.. It seems unsafe.
> > 
> > This is curious, I'm not sure why you would drop the lock there. Max?
> > 
> > bdrv_flush() calls would have to replaced with qcow2_write_caches() to
> > avoid a deadlock, but otherwise I don't see why we would want to drop
> > the lock.
> > 
> > Of course, this should only be called from qemu-img check, so in
> > practice it's probably not a bug.
> 
> Maybe I should have read all of this before replying...  Is
> qcow2_write_caches() all that we want?  I mean, bdrv_flush() also
> flushes the children (well, at least the file child right now).

You're probably right and we want to call qcow2_cache_flush() instead. I
forgot that we split these functions.

> OTOH qcow2_write_snapshots() probably does not need to take care of
> actually flushing @bs, does it?

I think it wants to get the right ordering to avoid corruption on
crashes, so we certainly do want to flush all the way down to the disk.

At least after qcow2_alloc_clusters(), it needs to flush bs itself; for
the second one, it could be enough to flush bs->file, but flushing bs
can't hurt.

Kevin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]