[Top][All Lists]

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

Re: [Qemu-devel] [PATCH] block/vdi: Fix locking for parallel requests

From: Max Reitz
Subject: Re: [Qemu-devel] [PATCH] block/vdi: Fix locking for parallel requests
Date: Wed, 18 Feb 2015 10:09:43 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0

On 2015-02-18 at 07:39, Kevin Wolf wrote:
Am 17.02.2015 um 22:33 hat Max Reitz geschrieben:
Concurrently modifying the bmap is not a good idea;
Why? I mean, the fact that this fixes something for you probably means
that there really is some piece of local state that is invalidated by
concurrent writes, but it's not obvious to me what it is.

One thing I can see is the following: The write operations for header and bmap are separate. Therefore, writing the header may succeed; then, when writing the bmap, something may yield and another instance of vdi_co_write() gets to run. That instance is still in the while loop, allocating a new block.

It modifies s->bmap[] and increases s->header.blocks_allocated; then tries to write. That yields and the first coroutine wakes up. It writes the bmap to the disk, which is now inconsistent with s->header.blocks_allocated, though. So that looks to me like it may break, although it in fact doesn't seem to be responsible for the problem at hand.

For that problem, always doing the unlock() after the bdrv_write() in the "if (!VDI_IS_ALLOCATED(bmap_entry))" path seems enough; although I cannot really explain how coroutines yielding from writing the data block interferes with other coroutines allocating new blocks.

However, in case you agree with my reasoning for the possible corruption (which I did not try to produce), this patch (which would fix that) would also happen to prevent the really appearing VDI problems, so...


What could obviously happen is that metadata is written before the data
is on the disk, but as we don't support backing files for VDI, this is
irrelvant. (And if it were relevant, it stil wouldn't be fixed by your
patch because the driver never flushes.)

this patch adds a
lock for it. See https://bugs.launchpad.net/qemu/+bug/1422307 for what
can go wrong without.

Signed-off-by: Max Reitz <address@hidden>

reply via email to

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