From: Kevin Wolf <kwolf@redhat.com>
To: Jamie Lokier <jamie@shareable.org>
Cc: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] qcow2: Bring synchronous read/write back to life
Date: Thu, 08 Oct 2009 17:47:47 +0200 [thread overview]
Message-ID: <4ACE09A3.9010909@redhat.com> (raw)
In-Reply-To: <20091008152831.GC29691@shareable.org>
Am 08.10.2009 17:28, schrieb Jamie Lokier:
> Kevin Wolf wrote:
>> The original bdrv_read would mean that we read some data
>> synchronously and that we won't be interrupted during this read. The
>> latter assumption is no longer true with the emulation function
>> which needs to use qemu_aio_poll and therefore allows the callback
>> of any other concurrent AIO request to be run during the read. Which
>> in turn means that (meta)data read earlier could have changed and be
>> invalid now.
>
> I'm not sure if I understand your description, specifically
> "(meta)data read earlier could have changed and be invalid now".
>
> Do you mean:
No, it's not exactly what I meant. But you're right, your scenario could
happen, too. If global state is changed in an AIO callback called during
bdrv_read/write (e.g. a new L2 table is loaded into the cache), bad
things are almost guaranteed to happen.
What I was thinking of is:
>
> Async call into qcow2 #2
> ------------------------
>
> issues a request with bdrv_read/write
>
> Async call into qcow2 #1
> ------------------------
> reads some metadata from memory (**)
#1 reads some metadata from disk (from the image file)
> does some calculations
> issues a request with bdrv_read/write
>
> the request completes
> updates some metadata in memory
#2 updates the metadata in the file
> async call finished with result
>
> the request completes
> updates some metadata in memory
> .... ERROR, memory isn't what it was at point (**)
#1 still uses the old metadata which it had loaded into memory
(specifically those parts on the stack).
Also, I was thinking of #2 as being a regular AIO write (no bdrv_write
involved), but again your version could work as well. As I said I don't
know yet which of them really happens.
Kevin
prev parent reply other threads:[~2009-10-08 15:49 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-10-08 13:02 [Qemu-devel] [PATCH] qcow2: Bring synchronous read/write back to life Kevin Wolf
2009-10-08 14:30 ` Anthony Liguori
2009-10-08 14:47 ` Kevin Wolf
2009-10-08 15:01 ` Anthony Liguori
2009-10-08 15:23 ` Kevin Wolf
2009-10-08 18:47 ` Mark McLoughlin
2009-10-08 15:28 ` Jamie Lokier
2009-10-08 15:47 ` Kevin Wolf [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4ACE09A3.9010909@redhat.com \
--to=kwolf@redhat.com \
--cc=jamie@shareable.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.