Am 27.02.2015 um 17:57 schrieb Stefan Hajnoczi: > On Fri, Feb 27, 2015 at 09:05:47AM -0500, Max Reitz wrote: >> Concurrently modifying the bmap does not seem to be a good idea; this patch >> adds >> a lock for it. See https://bugs.launchpad.net/qemu/+bug/1422307 for what >> can go wrong without. >> >> Cc: qemu-stable <qemu-sta...@nongnu.org> >> Signed-off-by: Max Reitz <mre...@redhat.com> >> --- >> v2: >> - Make the mutex cover vdi_co_write() completely [Kevin] >> - Add a TODO comment [Kevin] [...] >> If we don't know why bmap_lock works, it would be more approprate to >> take the same approach as VMDK and VHDX where there is a simply s->lock >> that protects all reads and writes. That way we know for sure there is >> no parallel I/O going on. >> >> (Since the problem is not understood, maybe reads in parallel with >> writes could also cause problems. Better to really do a coarse lock >> instead of just bmap_lock in write.) >> >> Stefan
block/vdi.c was never written for multi-threaded access, see my comment in the header of block/vdi.c: * The code is not thread safe (missing locks for changes in header and * block table, no problem with current QEMU). This was true in the past, but obviously later multi-threaded access was introduced for QEMU. Locking was added for qcow2 and other drivers in 2012 and 2013, but never for vdi. I must admit that I don't know which parts of the block filesystem drivers potentially run in parallel threads. Ideally there would be one or more test cases which test multi-threaded operations and which trigger a failure with the current vdi code. If I had a simple test scenario, I could have a look on the problem. The VMDK approach is fine as an intermediate work around, but please use conditional compilation to allow easy tests without coarse locks (and update the comments :-)). Regards Stefan (Weil)
