Re: [Qemu-devel] [PATCH 2/2] virtio-blk: disable write cache if not negotiated

2012-07-04 Thread Kevin Wolf
Am 03.07.2012 15:51, schrieb Paolo Bonzini:
 Il 03/07/2012 15:49, Kevin Wolf ha scritto:
 If the guest does not support flushes, we should run in writethrough mode.
 The setting is temporary until the next reset, so that for example the
 BIOS will run in writethrough mode while Linux will run with a writeback
 cache.

 VIRTIO_BLK_F_FLUSH has been introduced in Linux 2.6.32 (in 2009) and
 was backported to RHEL/CentOS 5.6 (in 2010).  The Windows drivers have
 two bugs, which I reported on the Red Hat Bugzilla as bugs 837321 and
 837324.  With these patches they will suffer a performance hit but
 gain correctness.

 Signed-off-by: Paolo Bonzini pbonz...@redhat.com
 I generally like the idea for a default, but doesn't this override even
 an explicit cache=writeback?
 
 Yes.  It doesn't override cache=unsafe though.

When the guest doesn't support flushes, cache=writeback is equivalent to
cache=unsafe, so if you want the old behaviour back you can switch to
cache=unsafe without additional risks.

We don't have a cache=directunsafe, though, so if you want to get the
old behaviour of cache=none back, you're out of luck. Not sure how
acceptable this is.

Irrespective of this concern I've come to the conclusion that I agree
and we actually must enforce this for non-unsafe mode, and not doing it
is a bug.

Kevin
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Qemu-devel] [PATCH 2/2] virtio-blk: disable write cache if not negotiated

2012-07-04 Thread Paolo Bonzini
Il 04/07/2012 12:16, Kevin Wolf ha scritto:
  Yes.  It doesn't override cache=unsafe though.
 When the guest doesn't support flushes, cache=writeback is equivalent to
 cache=unsafe, so if you want the old behaviour back you can switch to
 cache=unsafe without additional risks.
 
 We don't have a cache=directunsafe, though, so if you want to get the
 old behaviour of cache=none back, you're out of luck. Not sure how
 acceptable this is.

If we want to fix this, let's take the occasion to split the parameters
into cache=on/off (well, we have that already), flush=on/off, and a
device-side wce=on/off.

 Irrespective of this concern I've come to the conclusion that I agree
 and we actually must enforce this for non-unsafe mode, and not doing it
 is a bug.

Thanks!  Is that an Acked-by/Reviewed-by? :)

Paolo
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Qemu-devel] [PATCH 2/2] virtio-blk: disable write cache if not negotiated

2012-07-04 Thread Kevin Wolf
Am 04.07.2012 14:21, schrieb Paolo Bonzini:
 Il 04/07/2012 12:16, Kevin Wolf ha scritto:
 Yes.  It doesn't override cache=unsafe though.
 When the guest doesn't support flushes, cache=writeback is equivalent to
 cache=unsafe, so if you want the old behaviour back you can switch to
 cache=unsafe without additional risks.

 We don't have a cache=directunsafe, though, so if you want to get the
 old behaviour of cache=none back, you're out of luck. Not sure how
 acceptable this is.
 
 If we want to fix this, let's take the occasion to split the parameters
 into cache=on/off (well, we have that already), flush=on/off, and a
 device-side wce=on/off.

You're volunteering? Great! ;-)

 Irrespective of this concern I've come to the conclusion that I agree
 and we actually must enforce this for non-unsafe mode, and not doing it
 is a bug.
 
 Thanks!  Is that an Acked-by/Reviewed-by? :)

Before merging the patches (or actually this patch, I think patch 1 is
fairly independent), I'd like to hear more opinions on whether we need
the cache parameter split first. But as far as the hardware is
concerned, sure, take it as an Acked-by and go forward with the spec and
kernel side of things.

Kevin
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Qemu-devel] [PATCH 2/2] virtio-blk: disable write cache if not negotiated

2012-07-04 Thread Paolo Bonzini
Il 04/07/2012 14:50, Kevin Wolf ha scritto:
 Before merging the patches (or actually this patch, I think patch 1 is
 fairly independent),

Yes, it is.

 I'd like to hear more opinions on whether we need
 the cache parameter split first

Ok, let's discuss it next week on the KVM/QEMU call.  Getting the cache
parameter split before 1.2 is going to be hard, but who knows.

 But as far as the hardware is
 concerned, sure, take it as an Acked-by and go forward with the spec and
 kernel side of things.

Rusty already committed everything, so it's too late! :)  (Sorry for the
tabs vs. spaces BTW).

Paolo
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html