From 20180f303f6fd602ca8fd66bdd746916184177de Mon Sep 17 00:00:00 2001

From: Paolo Bonzini <pbonzini@redhat.com>

Date: Mon, 23 Sep 2013 17:08:02 +0200

Subject: [PATCH 04/29] virtio-blk: do not relay a previous driver's WCE configuration to the current

RH-Author: Paolo Bonzini <pbonzini@redhat.com>

Message-id: <1379956082-3646-3-git-send-email-pbonzini@redhat.com>

Patchwork-id: 54492

O-Subject: [RHEL 7.0 qemu-kvm PATCH 2/2] virtio-blk: do not relay a previous driver's WCE configuration to the current

Bugzilla: 1009993

RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>

RH-Acked-by: Fam Zheng <famz@redhat.com>

RH-Acked-by: Kevin Wolf <kwolf@redhat.com>

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.

Hence, whenever the guest is reset, the cache mode of the disk should

be reset to whatever was specified in the "-drive" option.  With this

change, the Linux virtio-blk driver finds that writeback caching is

enabled, and tells the block layer to send cache flush commands

appropriately.

Reported-by: Rusty Russell 

Cc: qemu-stable@nongnu.org

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

(cherry picked from commit ef5bc96268ceec64769617dc53b0ac3a20ff351c)

---

 hw/block/virtio-blk.c          | 24 ++++++++++++++++++++++--

 include/hw/virtio/virtio-blk.h |  1 +

 2 files changed, 23 insertions(+), 2 deletions(-)

Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>

---

 hw/block/virtio-blk.c          |   24 ++++++++++++++++++++++--

 include/hw/virtio/virtio-blk.h |    1 +

 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c

index cca0c77..ce1a523 100644

--- a/hw/block/virtio-blk.c

+++ b/hw/block/virtio-blk.c

@@ -460,9 +460,9 @@ static void virtio_blk_dma_restart_cb(void *opaque, int running,

 static void virtio_blk_reset(VirtIODevice *vdev)

 {

-#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE

     VirtIOBlock *s = VIRTIO_BLK(vdev);

+#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE

     if (s->dataplane) {

         virtio_blk_data_plane_stop(s->dataplane);

     }

@@ -473,6 +473,7 @@ static void virtio_blk_reset(VirtIODevice *vdev)

      * are per-device request lists.

      */

     bdrv_drain_all();

+    bdrv_set_enable_write_cache(s->bs, s->original_wce);

 }

 /* coalesce internal state, copy to pci i/o region 0

@@ -564,7 +565,25 @@ static void virtio_blk_set_status(VirtIODevice *vdev, uint8_t status)

     }

     features = vdev->guest_features;

-    bdrv_set_enable_write_cache(s->bs, !!(features & (1 << VIRTIO_BLK_F_WCE)));

+

+    /* A guest that supports VIRTIO_BLK_F_CONFIG_WCE must be able to send

+     * cache flushes.  Thus, the "auto writethrough" behavior is never

+     * necessary for guests that support the VIRTIO_BLK_F_CONFIG_WCE feature.

+     * Leaving it enabled would break the following sequence:

+     *

+     *     Guest started with "-drive cache=writethrough"

+     *     Guest sets status to 0

+     *     Guest sets DRIVER bit in status field

+     *     Guest reads host features (WCE=0, CONFIG_WCE=1)

+     *     Guest writes guest features (WCE=0, CONFIG_WCE=1)

+     *     Guest writes 1 to the WCE configuration field (writeback mode)

+     *     Guest sets DRIVER_OK bit in status field

+     *

+     * s->bs would erroneously be placed in writethrough mode.

+     */

+    if (!(features & (1 << VIRTIO_BLK_F_CONFIG_WCE))) {

+        bdrv_set_enable_write_cache(s->bs, !!(features & (1 << VIRTIO_BLK_F_WCE)));

+    }

 }

 static void virtio_blk_save(QEMUFile *f, void *opaque)

@@ -674,6 +693,7 @@ static int virtio_blk_device_init(VirtIODevice *vdev)

     }

     blkconf_serial(&blk->conf, &blk->serial);

+    s->original_wce = bdrv_enable_write_cache(blk->conf.bs);

     if (blkconf_geometry(&blk->conf, NULL, 65535, 255, 255) < 0) {

         return -1;

     }

diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h

index b87cf49..41885da 100644

--- a/include/hw/virtio/virtio-blk.h

+++ b/include/hw/virtio/virtio-blk.h

@@ -123,6 +123,7 @@ typedef struct VirtIOBlock {

     BlockConf *conf;

     VirtIOBlkConf blk;

     unsigned short sector_mask;

+    bool original_wce;

     VMChangeStateEntry *change;

 #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE

     Notifier migration_state_notifier;

-- 

1.7.1