On 2015-02-27 at 12:25, Stefan Weil wrote:
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.) Stefanblock/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.
I have one for you. See the attached ruby script. (If there are no "Pattern verification failed" messages, everything is good)
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 :-)).
Will a macro defined in vdi.c be enough? Max
test.rb
Description: application/ruby
