[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] block/vdi: Add locking for parallel requests
From: |
Stefan Weil |
Subject: |
Re: [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 :-)).
Regards
Stefan (Weil)
- [Qemu-devel] [PATCH v2] block/vdi: Add locking for parallel requests, Max Reitz, 2015/02/27
- Re: [Qemu-devel] [PATCH v2] block/vdi: Add locking for parallel requests, Stefan Hajnoczi, 2015/02/27
- Re: [Qemu-devel] [PATCH v2] block/vdi: Add locking for parallel requests, Max Reitz, 2015/02/27
- Re: [Qemu-devel] [PATCH v2] block/vdi: Add locking for parallel requests,
Stefan Weil <=
- Re: [Qemu-devel] [PATCH v2] block/vdi: Add locking for parallel requests, Max Reitz, 2015/02/27
- Re: [Qemu-devel] [PATCH v2] block/vdi: Add locking for parallel requests, Stefan Weil, 2015/02/27
- Re: [Qemu-devel] [PATCH v2] block/vdi: Add locking for parallel requests, Stefan Weil, 2015/02/27
- Re: [Qemu-devel] [PATCH v2] block/vdi: Add locking for parallel requests, Max Reitz, 2015/02/27
- Re: [Qemu-devel] [PATCH v2] block/vdi: Add locking for parallel requests, Stefan Weil, 2015/02/27
- Re: [Qemu-devel] [PATCH v2] block/vdi: Add locking for parallel requests, Max Reitz, 2015/02/27
- Re: [Qemu-devel] [PATCH v2] block/vdi: Add locking for parallel requests, Max Reitz, 2015/02/27
- Re: [Qemu-devel] [PATCH v2] block/vdi: Add locking for parallel requests, Stefan Weil, 2015/02/27
- Re: [Qemu-devel] [PATCH v2] block/vdi: Add locking for parallel requests, Max Reitz, 2015/02/27
- Re: [Qemu-devel] [PATCH v2] block/vdi: Add locking for parallel requests, Stefan Weil, 2015/02/27