[Top][All Lists]

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

Re: [Qemu-block] [Qemu-devel] [PATCH v2] block/vdi: Add locking for para

From: Stefan Weil
Subject: Re: [Qemu-block] [Qemu-devel] [PATCH v2] block/vdi: Add locking for parallel requests
Date: Fri, 27 Feb 2015 18:25:02 +0100
User-agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0

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 <address@hidden>
>> Signed-off-by: Max Reitz <address@hidden>
>> ---
>> 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 :-)).

Stefan (Weil)

reply via email to

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