qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: rusty@au1.ibm.com, qemu-devel@nongnu.org, stefanha@redhat.com,
	qemu-stable@nongnu.org
Subject: Re: [Qemu-devel] [PATCH 2/2] virtio-blk: do not relay a previous driver's WCE configuration to the current
Date: Fri, 20 Sep 2013 11:54:24 +0200	[thread overview]
Message-ID: <20130920095424.GD2800@dhcp-200-207.str.redhat.com> (raw)
In-Reply-To: <1379609334-20811-3-git-send-email-pbonzini@redhat.com>

Am 19.09.2013 um 18:48 hat Paolo Bonzini geschrieben:
> The following sequence happens:
> - the SeaBIOS virtio-blk driver does not support the WCE feature, which
> causes QEMU to disable writeback caching
> 
> - the Linux virtio-blk driver resets the device, finds WCE is available
> but writeback caching is disabled; tells block layer to not send cache
> flush commands
> 
> - the Linux virtio-blk driver sets the DRIVER_OK bit, which causes
> writeback caching to be re-enabled, but the Linux virtio-blk driver does
> not know of this side effect and cache flushes remain disabled
> 
> The bug is at the third step.  If the guest does know about CONFIG_WCE,
> QEMU should ignore the WCE feature's state.  The guest will control the
> cache mode solely using configuration space.  This change makes Linux
> do flushes correctly, but Linux will keep SeaBIOS's writethrough mode.

This sounds fishy. The solutions happens to make recent Linux kernels do
the right thing, but wouldn't drivers that don't know CONFIG_WCE still
fall into the same trap? I guess making a host feature flag dynamic was
a bad idea to start with.

Perhaps we should restrict the magic to disabling WCE in case the guest
doesn't have VIRTIO_BLK_F_WCE, but never allow it to enable WCE even
though we've already advertised that the host doesn't have WCE.

Kevin

  parent reply	other threads:[~2013-09-20  9:54 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-19 16:48 [Qemu-devel] [PATCH 0/2] Serious bugs in the handling of writeback caching Paolo Bonzini
2013-09-19 16:48 ` [Qemu-devel] [PATCH 1/2] blockdev: do not default cache.no-flush to true Paolo Bonzini
2013-09-19 17:33   ` Eric Blake
2013-09-19 16:48 ` [Qemu-devel] [PATCH 2/2] virtio-blk: do not relay a previous driver's WCE configuration to the current Paolo Bonzini
2013-09-19 17:42   ` Eric Blake
2013-09-20  9:54   ` Kevin Wolf [this message]
2013-09-20 10:28     ` Paolo Bonzini
2013-09-20 15:31   ` [Qemu-devel] [PATCH v2 " Paolo Bonzini
2013-09-20 17:28 ` [Qemu-devel] [PATCH 0/2] Serious bugs in the handling of writeback caching Stefan Hajnoczi

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=20130920095424.GD2800@dhcp-200-207.str.redhat.com \
    --to=kwolf@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-stable@nongnu.org \
    --cc=rusty@au1.ibm.com \
    --cc=stefanha@redhat.com \
    /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).