qemu-stable
[Top][All Lists]
Advanced

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

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


From: Max Reitz
Subject: Re: [Qemu-stable] [Qemu-devel] [PATCH v2] block/vdi: Add locking for parallel requests
Date: Fri, 27 Feb 2015 12:28:55 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0

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 <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.

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

Attachment: test.rb
Description: application/ruby


reply via email to

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