qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Anthony Liguori <anthony@codemonkey.ws>
Cc: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] qcow2: Bring synchronous read/write back to life
Date: Thu, 08 Oct 2009 16:47:00 +0200	[thread overview]
Message-ID: <4ACDFB64.1040106@redhat.com> (raw)
In-Reply-To: <4ACDF797.4010805@codemonkey.ws>

Am 08.10.2009 16:30, schrieb Anthony Liguori:
> Kevin Wolf wrote:
>> When the synchronous read and write functions were dropped, they were replaced
>> by generic emulation functions. Unfortunately, these emulation functions don't
>> provide the same semantics as the original functions did.
>>
>> 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.
> 
> Perhaps you could create a mechanism to freeze the qcow2 image by 
> queuing all completions within qcow2 until the image was unfrozen.  This 
> would have the same effect switching to synchronous read/write.
> 
> You may also have to queue new read/write requests...
> 
> Introducing sync read/write seems like a major step backwards to me.

Right, I was expecting your reaction. ;-) I do even agree that it's not
nice to have the synchronous functions back. But removing them caused a
regression, so the removal should be reverted until it is done right.

I just want to make clear that we're talking about data corruption here.
This is not just something that we can care about when we are bored some
time in the future.

For stable, I think taking this patch is the only reasonable thing to
do. Any other solution would be way too invasive. For master we can
discuss other solutions. However, I really don't feel like hacking
around it with a quick fix and breaking other stuff, so this takes a bit
more time. For the meantime I would prefer committing this to master, too.

By the way, I don't think queuing things in qcow2 is the right thing to
do. It probably wouldn't even help if, say, VMDK implemented AIO and I
used a VMDK image as a backing file for qcow2. The real solution is to
fix the generic bdrv_read/write emulation. Now we just need to find the
best way to do this.

Kevin

  reply	other threads:[~2009-10-08 14:48 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 [this message]
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

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=4ACDFB64.1040106@redhat.com \
    --to=kwolf@redhat.com \
    --cc=anthony@codemonkey.ws \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).