[Qemu-devel] [PATCH 1/4] block: add enable_write_cache flag

[Christoph Hellwig]

Add a enable_write_cache flag in the block driver state, and use it to
decide if we claim to have a volatile write cache that needs controlled
flushing from the guest.

Currently we only claim to have it when cache=none is specified.  While
that might seem wrong it actually is the case as we still have
outstanding block allocations and host drive caches to flush.  We do
not need to claim a write cache when we use cache=writethrough because
O_SYNC writes are guaranteed to have data on stable storage.

We would have to claim one for data=writeback to be safe, but for I will
follow Avi's opinion that it is a useless mode and should be our
dedicated unsafe mode.  If anyone disagrees please start the flame
thrower now and I will change it.  Otherwise a documentation patch will
follow to explicitly document data=writeback as unsafe.

Both scsi-disk and ide now use the new flage, changing from their
defaults of always off (ide) or always on (scsi-disk).

Signed-off-by: Christoph Hellwig <address@hidden>

Index: qemu-kvm/hw/scsi-disk.c
===================================================================
--- qemu-kvm.orig/hw/scsi-disk.c
+++ qemu-kvm/hw/scsi-disk.c
@@ -710,7 +710,9 @@ static int32_t scsi_send_command(SCSIDev
                 memset(p,0,20);
                 p[0] = 8;
                 p[1] = 0x12;
-                p[2] = 4; /* WCE */
+                if (bdrv_enable_write_cache(s->bdrv)) {
+                     p[2] = 4; /* WCE */
+                }
                 p += 20;
             }
             if ((page == 0x3f || page == 0x2a)
Index: qemu-kvm/block.c
===================================================================
--- qemu-kvm.orig/block.c
+++ qemu-kvm/block.c
@@ -408,6 +408,16 @@ int bdrv_open2(BlockDriverState *bs, con
     }
     bs->drv = drv;
     bs->opaque = qemu_mallocz(drv->instance_size);
+
+    /*
+     * Yes, BDRV_O_NOCACHE aka O_DIRECT means we have to present a
+     * write cache to the guest.  We do need the fdatasync to flush
+     * out transactions for block allocations, and we maybe have a
+     * volatile write cache in our backing device to deal with.
+     */
+    if (flags & BDRV_O_NOCACHE)
+        bs->enable_write_cache = 1;
+
     /* Note: for compatibility, we open disk image files as RDWR, and
        RDONLY as fallback */
     if (!(flags & BDRV_O_FILE))
@@ -918,6 +928,11 @@ int bdrv_is_sg(BlockDriverState *bs)
     return bs->sg;
 }
 
+int bdrv_enable_write_cache(BlockDriverState *bs)
+{
+    return bs->enable_write_cache;
+}
+
 /* XXX: no longer used */
 void bdrv_set_change_cb(BlockDriverState *bs,
                         void (*change_cb)(void *opaque), void *opaque)
Index: qemu-kvm/block_int.h
===================================================================
--- qemu-kvm.orig/block_int.h
+++ qemu-kvm/block_int.h
@@ -152,6 +152,9 @@ struct BlockDriverState {
     /* the memory alignment required for the buffers handled by this driver */
     int buffer_alignment;
 
+    /* do we need to tell the quest if we have a volatile write cache? */
+    int enable_write_cache;
+
     /* NOTE: the following infos are only hints for real hardware
        drivers. They are not used by the block driver */
     int cyls, heads, secs, translation;
Index: qemu-kvm/block.h
===================================================================
--- qemu-kvm.orig/block.h
+++ qemu-kvm/block.h
@@ -120,6 +120,7 @@ int bdrv_get_translation_hint(BlockDrive
 int bdrv_is_removable(BlockDriverState *bs);
 int bdrv_is_read_only(BlockDriverState *bs);
 int bdrv_is_sg(BlockDriverState *bs);
+int bdrv_enable_write_cache(BlockDriverState *bs);
 int bdrv_is_inserted(BlockDriverState *bs);
 int bdrv_media_changed(BlockDriverState *bs);
 int bdrv_is_locked(BlockDriverState *bs);
Index: qemu-kvm/hw/ide/core.c
===================================================================
--- qemu-kvm.orig/hw/ide/core.c
+++ qemu-kvm/hw/ide/core.c
@@ -148,8 +148,11 @@ static void ide_identify(IDEState *s)
     put_le16(p + 83, (1 << 14) | (1 << 13) | (1 <<12) | (1 << 10));
     /* 14=set to 1, 1=SMART self test, 0=SMART error logging */
     put_le16(p + 84, (1 << 14) | 0);
-    /* 14 = NOP supported, 0=SMART feature set enabled */
-    put_le16(p + 85, (1 << 14) | 1);
+    /* 14 = NOP supported, 5=WCACHE enabled, 0=SMART feature set enabled */
+    if (bdrv_enable_write_cache(s->bs))
+         put_le16(p + 85, (1 << 14) | (1 << 5) | 1);
+    else
+         put_le16(p + 85, (1 << 14) | 1);
     /* 13=flush_cache_ext,12=flush_cache,10=lba48 */
     put_le16(p + 86, (1 << 14) | (1 << 13) | (1 <<12) | (1 << 10));
     /* 14=set to 1, 1=smart self test, 0=smart error logging */