[Qemu-devel] [PATCH 1/3] make bdrv_flush pay attention to errors

[Qemu-devel] [PATCH 1/3] make bdrv_flush pay attention to errors

[Ian Jackson]

We change bdrv_flush, and all of its implementations in the block
backends, to return int rather than void.  This enables the IDE and
SCSI controllers to report errors from FLUSH CACHE commands.

Cherry picked from qemu-xen adc804735154e1945e2d104ced2b40d6fc4dc07c.
Conflicts:
	block.h
	hw/ide.c

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 block-qcow.c      |    4 ++--
 block-qcow2.c     |    4 ++--
 block-raw-posix.c |    6 ++++--
 block-raw-win32.c |    8 ++++++--
 block-vmdk.c      |    4 ++--
 block.c           |   12 +++++++-----
 block.h           |    2 +-
 block_int.h       |    2 +-
 hw/ide.c          |    7 +++++--
 hw/scsi-disk.c    |    8 +++++++-
 10 files changed, 37 insertions(+), 20 deletions(-)

diff --git a/block-qcow.c b/block-qcow.c
index 1fecf30..b5bfce6 100644
--- a/block-qcow.c
+++ b/block-qcow.c
@@ -868,10 +868,10 @@ static int qcow_write_compressed(BlockDriverState *bs, int64_t sector_num,
     return 0;
 }
 
-static void qcow_flush(BlockDriverState *bs)
+static int qcow_flush(BlockDriverState *bs)
 {
     BDRVQcowState *s = bs->opaque;
-    bdrv_flush(s->hd);
+    return bdrv_flush(s->hd);
 }
 
 static int qcow_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
diff --git a/block-qcow2.c b/block-qcow2.c
index b9f1688..07c64ce 100644
--- a/block-qcow2.c
+++ b/block-qcow2.c
@@ -1600,10 +1600,10 @@ static int qcow_write_compressed(BlockDriverState *bs, int64_t sector_num,
     return 0;
 }
 
-static void qcow_flush(BlockDriverState *bs)
+static int qcow_flush(BlockDriverState *bs)
 {
     BDRVQcowState *s = bs->opaque;
-    bdrv_flush(s->hd);
+    return bdrv_flush(s->hd);
 }
 
 static int qcow_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
diff --git a/block-raw-posix.c b/block-raw-posix.c
index 96edab4..5fdd6e9 100644
--- a/block-raw-posix.c
+++ b/block-raw-posix.c
@@ -837,10 +837,12 @@ static int raw_create(const char *filename, int64_t total_size,
     return 0;
 }
 
-static void raw_flush(BlockDriverState *bs)
+static int raw_flush(BlockDriverState *bs)
 {
     BDRVRawState *s = bs->opaque;
-    fsync(s->fd);
+    if (fsync(s->fd))
+        return errno;
+    return 0;
 }
 
 BlockDriver bdrv_raw = {
diff --git a/block-raw-win32.c b/block-raw-win32.c
index 43d3f6c..b86c66e 100644
--- a/block-raw-win32.c
+++ b/block-raw-win32.c
@@ -269,10 +269,14 @@ static void raw_aio_cancel(BlockDriverAIOCB *blockacb)
 }
 #endif /* #if 0 */
 
-static void raw_flush(BlockDriverState *bs)
+static int raw_flush(BlockDriverState *bs)
 {
     BDRVRawState *s = bs->opaque;
-    FlushFileBuffers(s->hfile);
+    int ret;
+    ret = FlushFileBuffers(s->hfile);
+    if (ret)
+	return -EIO;
+    return 0;
 }
 
 static void raw_close(BlockDriverState *bs)
diff --git a/block-vmdk.c b/block-vmdk.c
index e85bd42..7f9a9a8 100644
--- a/block-vmdk.c
+++ b/block-vmdk.c
@@ -813,10 +813,10 @@ static void vmdk_close(BlockDriverState *bs)
     bdrv_delete(s->hd);
 }
 
-static void vmdk_flush(BlockDriverState *bs)
+static int vmdk_flush(BlockDriverState *bs)
 {
     BDRVVmdkState *s = bs->opaque;
-    bdrv_flush(s->hd);
+    return bdrv_flush(s->hd);
 }
 
 BlockDriver bdrv_vmdk = {
diff --git a/block.c b/block.c
index db8244c..e5aa401 100644
--- a/block.c
+++ b/block.c
@@ -875,12 +875,14 @@ const char *bdrv_get_device_name(BlockDriverState *bs)
     return bs->device_name;
 }
 
-void bdrv_flush(BlockDriverState *bs)
+int bdrv_flush(BlockDriverState *bs)
 {
-    if (bs->drv->bdrv_flush)
-        bs->drv->bdrv_flush(bs);
-    if (bs->backing_hd)
-        bdrv_flush(bs->backing_hd);
+    int ret = 0;
+    if (bs->drv->bdrv_flush) 
+        ret = bs->drv->bdrv_flush(bs);
+    if (!ret && bs->backing_hd)
+        ret = bdrv_flush(bs->backing_hd);
+    return ret;
 }
 
 /*
diff --git a/block.h b/block.h
index fa741b5..510818b 100644
--- a/block.h
+++ b/block.h
@@ -99,7 +99,7 @@ void qemu_aio_wait_end(void);
 int qemu_key_check(BlockDriverState *bs, const char *name);
 
 /* Ensure contents are flushed to disk.  */
-void bdrv_flush(BlockDriverState *bs);
+int bdrv_flush(BlockDriverState *bs);
 int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
 	int *pnum);
 
diff --git a/block_int.h b/block_int.h
index 137000e..796ce29 100644
--- a/block_int.h
+++ b/block_int.h
@@ -42,7 +42,7 @@ struct BlockDriver {
     void (*bdrv_close)(BlockDriverState *bs);
     int (*bdrv_create)(const char *filename, int64_t total_sectors,
                        const char *backing_file, int flags);
-    void (*bdrv_flush)(BlockDriverState *bs);
+    int (*bdrv_flush)(BlockDriverState *bs);
     int (*bdrv_is_allocated)(BlockDriverState *bs, int64_t sector_num,
                              int nb_sectors, int *pnum);
     int (*bdrv_set_key)(BlockDriverState *bs, const char *key);
diff --git a/hw/ide.c b/hw/ide.c
index fd3a055..6b73f41 100644
--- a/hw/ide.c
+++ b/hw/ide.c
@@ -2014,6 +2014,7 @@ static void ide_ioport_write(void *opaque, uint32_t addr, uint32_t val)
     IDEState *s;
     int unit, n;
     int lba48 = 0;
+    int ret;
 
 #ifdef DEBUG_IDE
     printf("IDE: write addr=0x%x val=0x%02x\n", addr, val);
@@ -2276,8 +2277,10 @@ static void ide_ioport_write(void *opaque, uint32_t addr, uint32_t val)
             break;
         case WIN_FLUSH_CACHE:
         case WIN_FLUSH_CACHE_EXT:
-            if (s->bs)
-                bdrv_flush(s->bs);
+            if (s->bs) {
+                ret = bdrv_flush(s->bs);
+		if (ret) goto abort_cmd;
+	    }
 	    s->status = READY_STAT | SEEK_STAT;
             ide_set_irq(s);
             break;
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 16b3215..8c40cbf 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -293,6 +293,7 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag,
     uint8_t command;
     uint8_t *outbuf;
     SCSIRequest *r;
+    int ret;
 
     command = buf[0];
     r = scsi_find_request(s, tag);
@@ -689,7 +690,12 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag,
         break;
     case 0x35:
         DPRINTF("Synchronise cache (sector %d, count %d)\n", lba, len);
-        bdrv_flush(s->bdrv);
+        ret = bdrv_flush(s->bdrv);
+        if (ret) {
+            DPRINTF("IO error on bdrv_flush\n");
+            scsi_command_complete(r, SENSE_HARDWARE_ERROR);
+            return 0;
+        }
         break;
     case 0x43:
         {
-- 
1.4.4.4

[Qemu-devel] [PATCH 2/3] Perform emulated IDE flushes asynchronously.

[Ian Jackson]

We arrange for the WIN_FLUSH_CACHE and WIN_FLUSH_CACHE_EXT
commands to use a new bdrv_aio_flush facility.

If there is an error, the ATA-7 spec says that we are supposed to know
which is the first block whose flush failed and leave that in the
block offset registers.  However since we are using f(data)sync that's
not possible for us.  There is sadly no way for us to report the error
which won't encourage the guest to try to understand what went wrong
and then do the flush again expecting the remaining blocks to be
written (as specified by ATA-7).

So if the asynchronous flush fails, we kill the disk by detaching
->bs.  This makes it vanish: we don't generate any more interrupts,
leave status set to busy, and ignore future commands (and discard any
in-flight IO).  Alan Cox reports that this will probably induce the
best available behaviour in guests (retry for a while and then give
up).  Fine-grained error reporting is available if the guest turns off
the write cache.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Modified-by: Ian Jackson <ian.jackson@eu.citrix.com>
Signed-off-by: Kouya Shimura <kouya@jp.fujitsu.com>

Cherry picked from qemu-xen
	d1e5cc49395831cb9c23e00c37898cf943c1d4be
Conflicts:
	hw/ide.c
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 block-qcow.c      |    8 +++++++
 block-qcow2.c     |    8 +++++++
 block-raw-posix.c |   16 +++++++++++++++
 block.c           |   39 ++++++++++++++++++++++++++++++++++++++
 block.h           |    2 +
 block_int.h       |    2 +
 hw/ide.c          |   54 +++++++++++++++++++++++++++++++++++++++++++++-------
 7 files changed, 121 insertions(+), 8 deletions(-)

diff --git a/block-qcow.c b/block-qcow.c
index b5bfce6..f0043a0 100644
--- a/block-qcow.c
+++ b/block-qcow.c
@@ -720,6 +720,13 @@ static void qcow_aio_cancel(BlockDriverAIOCB *blockacb)
     qemu_aio_release(acb);
 }
 
+static BlockDriverAIOCB *qcow_aio_flush(BlockDriverState *bs,
+        BlockDriverCompletionFunc *cb, void *opaque)
+{
+    BDRVQcowState *s = bs->opaque;
+    return bdrv_aio_flush(s->hd, cb, opaque);
+}
+
 static void qcow_close(BlockDriverState *bs)
 {
     BDRVQcowState *s = bs->opaque;
@@ -898,6 +905,7 @@ BlockDriver bdrv_qcow = {
     .bdrv_aio_read = qcow_aio_read,
     .bdrv_aio_write = qcow_aio_write,
     .bdrv_aio_cancel = qcow_aio_cancel,
+    .bdrv_aio_flush = qcow_aio_flush,
     .aiocb_size = sizeof(QCowAIOCB),
     .bdrv_write_compressed = qcow_write_compressed,
     .bdrv_get_info = qcow_get_info,
diff --git a/block-qcow2.c b/block-qcow2.c
index 07c64ce..71df9a9 100644
--- a/block-qcow2.c
+++ b/block-qcow2.c
@@ -1377,6 +1377,13 @@ static void qcow_aio_cancel(BlockDriverAIOCB *blockacb)
     qemu_aio_release(acb);
 }
 
+static BlockDriverAIOCB *qcow_aio_flush(BlockDriverState *bs,
+        BlockDriverCompletionFunc *cb, void *opaque)
+{
+    BDRVQcowState *s = bs->opaque;
+    return bdrv_aio_flush(s->hd, cb, opaque);
+}
+
 static void qcow_close(BlockDriverState *bs)
 {
     BDRVQcowState *s = bs->opaque;
@@ -2613,6 +2620,7 @@ BlockDriver bdrv_qcow2 = {
     .bdrv_aio_read = qcow_aio_read,
     .bdrv_aio_write = qcow_aio_write,
     .bdrv_aio_cancel = qcow_aio_cancel,
+    .bdrv_aio_flush = qcow_aio_flush,
     .aiocb_size = sizeof(QCowAIOCB),
     .bdrv_write_compressed = qcow_write_compressed,
 
diff --git a/block-raw-posix.c b/block-raw-posix.c
index 5fdd6e9..43ae293 100644
--- a/block-raw-posix.c
+++ b/block-raw-posix.c
@@ -1062,6 +1062,21 @@ static int fd_open(BlockDriverState *bs)
 {
     return 0;
 }
+
+static BlockDriverAIOCB *raw_aio_flush(BlockDriverState *bs,
+        BlockDriverCompletionFunc *cb, void *opaque)
+{
+    RawAIOCB *acb;
+
+    acb = raw_aio_setup(bs, 0, NULL, 0, cb, opaque);
+    if (!acb)
+        return NULL;
+    if (aio_fsync(O_SYNC, &acb->aiocb) < 0) {
+        qemu_aio_release(acb);
+        return NULL;
+    }
+    return &acb->common;
+}
 #endif
 
 #if defined(__linux__)
@@ -1214,6 +1229,7 @@ BlockDriver bdrv_host_device = {
     .bdrv_aio_read = raw_aio_read,
     .bdrv_aio_write = raw_aio_write,
     .bdrv_aio_cancel = raw_aio_cancel,
+    .bdrv_aio_flush = raw_aio_flush,
     .aiocb_size = sizeof(RawAIOCB),
 #endif
     .bdrv_pread = raw_pread,
diff --git a/block.c b/block.c
index e5aa401..127430c 100644
--- a/block.c
+++ b/block.c
@@ -50,6 +50,8 @@ static BlockDriverAIOCB *bdrv_aio_read_em(BlockDriverState *bs,
 static BlockDriverAIOCB *bdrv_aio_write_em(BlockDriverState *bs,
         int64_t sector_num, const uint8_t *buf, int nb_sectors,
         BlockDriverCompletionFunc *cb, void *opaque);
+static BlockDriverAIOCB *bdrv_aio_flush_em(BlockDriverState *bs,
+        BlockDriverCompletionFunc *cb, void *opaque);
 static void bdrv_aio_cancel_em(BlockDriverAIOCB *acb);
 static int bdrv_read_em(BlockDriverState *bs, int64_t sector_num,
                         uint8_t *buf, int nb_sectors);
@@ -136,6 +138,8 @@ static void bdrv_register(BlockDriver *bdrv)
         bdrv->bdrv_read = bdrv_read_em;
         bdrv->bdrv_write = bdrv_write_em;
     }
+    if (!bdrv->bdrv_aio_flush)
+        bdrv->bdrv_aio_flush = bdrv_aio_flush_em;
     bdrv->next = first_drv;
     first_drv = bdrv;
 }
@@ -1189,6 +1193,17 @@ void bdrv_aio_cancel(BlockDriverAIOCB *acb)
     drv->bdrv_aio_cancel(acb);
 }
 
+BlockDriverAIOCB *bdrv_aio_flush(BlockDriverState *bs, 
+                                 BlockDriverCompletionFunc *cb, void *opaque)
+{
+    BlockDriver *drv = bs->drv;
+
+    if (!drv)
+        return NULL;
+
+    return drv->bdrv_aio_flush(bs, cb, opaque);
+}
+
 
 /**************************************************************/
 /* async block device emulation */
@@ -1214,6 +1229,15 @@ static BlockDriverAIOCB *bdrv_aio_write_em(BlockDriverState *bs,
     return NULL;
 }
 
+static BlockDriverAIOCB *bdrv_aio_flush_em(BlockDriverState *bs,
+        BlockDriverCompletionFunc *cb, void *opaque)
+{
+    int ret;
+    ret = bdrv_flush(bs);
+    cb(opaque, ret);
+    return NULL;
+}
+
 static void bdrv_aio_cancel_em(BlockDriverAIOCB *acb)
 {
 }
@@ -1257,6 +1281,21 @@ static BlockDriverAIOCB *bdrv_aio_write_em(BlockDriverState *bs,
     return &acb->common;
 }
 
+static BlockDriverAIOCB *bdrv_aio_flush_em(BlockDriverState *bs,
+        BlockDriverCompletionFunc *cb, void *opaque)
+{
+    BlockDriverAIOCBSync *acb;
+    int ret;
+
+    acb = qemu_aio_get(bs, cb, opaque);
+    if (!acb->bh)
+        acb->bh = qemu_bh_new(bdrv_aio_bh_cb, acb);
+    ret = bdrv_flush(bs);
+    acb->ret = ret;
+    qemu_bh_schedule(acb->bh);
+    return &acb->common;
+}
+
 static void bdrv_aio_cancel_em(BlockDriverAIOCB *blockacb)
 {
     BlockDriverAIOCBSync *acb = (BlockDriverAIOCBSync *)blockacb;
diff --git a/block.h b/block.h
index 510818b..05a3de8 100644
--- a/block.h
+++ b/block.h
@@ -87,6 +87,8 @@ BlockDriverAIOCB *bdrv_aio_read(BlockDriverState *bs, int64_t sector_num,
 BlockDriverAIOCB *bdrv_aio_write(BlockDriverState *bs, int64_t sector_num,
                                  const uint8_t *buf, int nb_sectors,
                                  BlockDriverCompletionFunc *cb, void *opaque);
+BlockDriverAIOCB *bdrv_aio_flush(BlockDriverState *bs, 
+                                 BlockDriverCompletionFunc *cb, void *opaque);
 void bdrv_aio_cancel(BlockDriverAIOCB *acb);
 
 void qemu_aio_init(void);
diff --git a/block_int.h b/block_int.h
index 796ce29..bda0eab 100644
--- a/block_int.h
+++ b/block_int.h
@@ -55,6 +55,8 @@ struct BlockDriver {
         int64_t sector_num, const uint8_t *buf, int nb_sectors,
         BlockDriverCompletionFunc *cb, void *opaque);
     void (*bdrv_aio_cancel)(BlockDriverAIOCB *acb);
+    BlockDriverAIOCB *(*bdrv_aio_flush)(BlockDriverState *bs,
+        BlockDriverCompletionFunc *cb, void *opaque);
     int aiocb_size;
 
     const char *protocol_name;
diff --git a/hw/ide.c b/hw/ide.c
index 6b73f41..a3b9e6e 100644
--- a/hw/ide.c
+++ b/hw/ide.c
@@ -748,6 +748,7 @@ static inline void ide_dma_submit_check(IDEState *s,
 static inline void ide_set_irq(IDEState *s)
 {
     BMDMAState *bm = s->bmdma;
+    if (!s->bs) return; /* ouch! (see ide_flush_cb) */
     if (!(s->cmd & IDE_CMD_DISABLE_IRQ)) {
         if (bm) {
             bm->status |= BM_STATUS_INT;
@@ -927,6 +928,8 @@ static void ide_read_dma_cb(void *opaque, int ret)
 	return;
     }
 
+    if (!s->bs) return; /* ouch! (see ide_flush_cb) */
+
     n = s->io_buffer_size >> 9;
     sector_num = ide_get_sector(s);
     if (n > 0) {
@@ -1038,6 +1041,8 @@ static void ide_write_dma_cb(void *opaque, int ret)
 	return;
     }
 
+    if (!s->bs) return; /* ouch! (see ide_flush_cb) */
+
     n = s->io_buffer_size >> 9;
     sector_num = ide_get_sector(s);
     if (n > 0) {
@@ -1084,6 +1089,39 @@ static void ide_sector_write_dma(IDEState *s)
     ide_dma_start(s, ide_write_dma_cb);
 }
 
+static void ide_device_utterly_broken(IDEState *s) {
+    s->status |= BUSY_STAT;
+    s->bs = NULL;
+    /* This prevents all future commands from working.  All of the
+     * asynchronous callbacks (and ide_set_irq, as a safety measure)
+     * check to see whether this has happened and bail if so.
+     */
+}
+
+static void ide_flush_cb(void *opaque, int ret)
+{
+    IDEState *s = opaque;
+
+    if (!s->bs) return; /* ouch! (see ide_flush_cb) */
+
+    if (ret) {
+        /* We are completely doomed.  The IDE spec does not permit us
+	 * to return an error from a flush except via a protocol which
+	 * requires us to say where the error is and which
+	 * contemplates the guest repeating the flush attempt to
+	 * attempt flush the remaining data.  We can't support that
+	 * because f(data)sync (which is what the block drivers use
+	 * eventually) doesn't report the necessary information or
+	 * give us the necessary control.  So we make the disk vanish.
+	 */
+	ide_device_utterly_broken(s);
+	return;
+    }
+    else
+        s->status = READY_STAT | SEEK_STAT;
+    ide_set_irq(s);
+}
+
 static void ide_atapi_cmd_ok(IDEState *s)
 {
     s->error = 0;
@@ -1310,6 +1348,8 @@ static void ide_atapi_cmd_read_dma_cb(void *opaque, int ret)
     IDEState *s = bm->ide_if;
     int data_offset, n;
 
+    if (!s->bs) return; /* ouch! (see ide_flush_cb) */
+
     if (ret < 0) {
         ide_atapi_io_error(s, ret);
         goto eot;
@@ -1973,6 +2013,8 @@ static void cdrom_change_cb(void *opaque)
     IDEState *s = opaque;
     uint64_t nb_sectors;
 
+    if (!s->bs) return; /* ouch! (see ide_flush_cb) */
+
     /* XXX: send interrupt too */
     bdrv_get_geometry(s->bs, &nb_sectors);
     s->nb_sectors = nb_sectors;
@@ -2081,8 +2123,8 @@ static void ide_ioport_write(void *opaque, uint32_t addr, uint32_t val)
         printf("ide: CMD=%02x\n", val);
 #endif
         s = ide_if->cur_drive;
-        /* ignore commands to non existant slave */
-        if (s != ide_if && !s->bs)
+	/* ignore commands to non existant device */
+        if (!s->bs)
             break;
 
         /* Only DEVICE RESET is allowed while BSY or/and DRQ are set */
@@ -2277,12 +2319,8 @@ static void ide_ioport_write(void *opaque, uint32_t addr, uint32_t val)
             break;
         case WIN_FLUSH_CACHE:
         case WIN_FLUSH_CACHE_EXT:
-            if (s->bs) {
-                ret = bdrv_flush(s->bs);
-		if (ret) goto abort_cmd;
-	    }
-	    s->status = READY_STAT | SEEK_STAT;
-            ide_set_irq(s);
+	    s->status = BUSY_STAT;
+	    bdrv_aio_flush(s->bs, ide_flush_cb, s);
             break;
         case WIN_STANDBY:
         case WIN_STANDBY2:
-- 
1.4.4.4

[Qemu-devel] [PATCH 3/3] make write cacheing controllable by guest

[Ian Jackson]

This patch implements the ATA write cache feature.  This enables a
guest to control, in the standard way, whether disk writes are
immediately committed to disk before the IDE command completes, or may
be buffered in the host.

In this patch, by default buffering is off, which provides better
reliability but may have a performance impact.  It would be
straightforward to change the default, or perhaps offer a command-line
option, if that would be preferred.

This patch is derived from one which was originally submitted to the
Xen tree by Rik van Riel <riel@redhat.com> and includes code to save
the write_cache setting from Samuel Thibault.  Thanks also to Paul
Brook for pointing out a bug, fixed in Xen and in this change.

From: Rik van Riel <riel@redhat.com>
Signed-off-by: Christian Limpach <Christian.Limpach@xensource.com>
Signed-off-by: Samuel Thibault <samuel.thibault@eu.citrix.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

Cherry picked from the following commits to qemu-xen-unstable:
 a9c94b06d240709c99966722cae655c5aa6771bf ide write_cache ... bdrv_aio_flush
 76a7df8cfdc7a8c2d89a181ab5196490f800f733 advertise write cache enabled ...
 f881e2f3b58647a6a132bd0d944358c89887b7a2 ide write_cache saving ... fixes
 9c2f4d468e1653f8f73a215ca2b375875c68748d Move WIN_SETFEATURES 0x02 and ...
 d1e5cc49395831cb9c23e00c37898cf943c1d4be make write cacheing controllable ...
with no conflicts.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 hw/ide.c |   83 +++++++++++++++++++++++++++++++++++++++++++------------------
 1 files changed, 58 insertions(+), 25 deletions(-)

diff --git a/hw/ide.c b/hw/ide.c
index a3b9e6e..b5c9a11 100644
--- a/hw/ide.c
+++ b/hw/ide.c
@@ -384,6 +384,7 @@ typedef struct IDEState {
     PCIDevice *pci_dev;
     struct BMDMAState *bmdma;
     int drive_serial;
+    uint8_t write_cache;
     /* ide regs */
     uint8_t feature;
     uint8_t error;
@@ -577,11 +578,12 @@ static void ide_identify(IDEState *s)
     put_le16(p + 68, 120);
     put_le16(p + 80, 0xf0); /* ata3 -> ata6 supported */
     put_le16(p + 81, 0x16); /* conforms to ata5 */
-    put_le16(p + 82, (1 << 14));
+    /* 14=nop 5=write_cache */
+    put_le16(p + 82, (1 << 14) | (1 << 5));
     /* 13=flush_cache_ext,12=flush_cache,10=lba48 */
     put_le16(p + 83, (1 << 14) | (1 << 13) | (1 <<12) | (1 << 10));
     put_le16(p + 84, (1 << 14));
-    put_le16(p + 85, (1 << 14));
+    put_le16(p + 85, (1 << 14) | (s->write_cache << 5));
     /* 13=flush_cache_ext,12=flush_cache,10=lba48 */
     put_le16(p + 86, (1 << 14) | (1 << 13) | (1 <<12) | (1 << 10));
     put_le16(p + 87, (1 << 14));
@@ -916,6 +918,14 @@ static int dma_buf_rw(BMDMAState *bm, int is_write)
     return 1;
 }
 
+static void ide_dma_eot(BMDMAState *bm) {
+    bm->status &= ~BM_STATUS_DMAING;
+    bm->status |= BM_STATUS_INT;
+    bm->dma_cb = NULL;
+    bm->ide_if = NULL;
+    bm->aiocb = NULL;
+}
+
 static void ide_read_dma_cb(void *opaque, int ret)
 {
     BMDMAState *bm = opaque;
@@ -945,11 +955,7 @@ static void ide_read_dma_cb(void *opaque, int ret)
         s->status = READY_STAT | SEEK_STAT;
         ide_set_irq(s);
     eot:
-        bm->status &= ~BM_STATUS_DMAING;
-        bm->status |= BM_STATUS_INT;
-        bm->dma_cb = NULL;
-        bm->ide_if = NULL;
-        bm->aiocb = NULL;
+	ide_dma_eot(bm);
         return;
     }
 
@@ -995,6 +1001,9 @@ static void ide_sector_write(IDEState *s)
     if (n > s->req_nb_sectors)
         n = s->req_nb_sectors;
     ret = bdrv_write(s->bs, sector_num, s->io_buffer, n);
+    if (ret == 0 && !s->write_cache) {
+	ret = bdrv_flush(s->bs);
+    }
     if (ret != 0) {
 	ide_rw_error(s);
 	return;
@@ -1029,6 +1038,20 @@ static void ide_sector_write(IDEState *s)
     }
 }
 
+static void ide_write_flush_cb(void *opaque, int ret) {
+    BMDMAState *bm = opaque;
+    IDEState *s = bm->ide_if;
+
+    if (ret != 0) {
+	ide_dma_error(s);
+	return;
+    }
+    s->status = READY_STAT | SEEK_STAT;
+    ide_set_irq(s);
+    ide_dma_eot(bm);
+    return;
+}
+
 static void ide_write_dma_cb(void *opaque, int ret)
 {
     BMDMAState *bm = opaque;
@@ -1053,14 +1076,14 @@ static void ide_write_dma_cb(void *opaque, int ret)
 
     /* end of transfer ? */
     if (s->nsector == 0) {
+	if (!s->write_cache) {
+	    bdrv_aio_flush(s->bs, ide_write_flush_cb, bm);
+	    return;
+	}
         s->status = READY_STAT | SEEK_STAT;
         ide_set_irq(s);
     eot:
-        bm->status &= ~BM_STATUS_DMAING;
-        bm->status |= BM_STATUS_INT;
-        bm->dma_cb = NULL;
-        bm->ide_if = NULL;
-        bm->aiocb = NULL;
+	ide_dma_eot(bm);
         return;
     }
 
@@ -1382,11 +1405,7 @@ static void ide_atapi_cmd_read_dma_cb(void *opaque, int ret)
         s->nsector = (s->nsector & ~7) | ATAPI_INT_REASON_IO | ATAPI_INT_REASON_CD;
         ide_set_irq(s);
     eot:
-        bm->status &= ~BM_STATUS_DMAING;
-        bm->status |= BM_STATUS_INT;
-        bm->dma_cb = NULL;
-        bm->ide_if = NULL;
-        bm->aiocb = NULL;
+	ide_dma_eot(bm);
         return;
     }
 
@@ -2266,8 +2285,6 @@ static void ide_ioport_write(void *opaque, uint32_t addr, uint32_t val)
             switch(s->feature) {
             case 0xcc: /* reverting to power-on defaults enable */
             case 0x66: /* reverting to power-on defaults disable */
-            case 0x02: /* write cache enable */
-            case 0x82: /* write cache disable */
             case 0xaa: /* read look-ahead enable */
             case 0x55: /* read look-ahead disable */
             case 0x05: /* set advanced power management mode */
@@ -2281,6 +2298,18 @@ static void ide_ioport_write(void *opaque, uint32_t addr, uint32_t val)
                 s->status = READY_STAT | SEEK_STAT;
                 ide_set_irq(s);
                 break;
+            case 0x02: /* write cache enable */
+                s->write_cache = 1;
+                s->status = READY_STAT | SEEK_STAT;
+                ide_set_irq(s);
+                break;
+            case 0x82: /* write cache disable */
+                s->write_cache = 0;
+                ret = bdrv_flush(s->bs);
+		if (ret != 0) goto abort_cmd;
+                s->status = READY_STAT | SEEK_STAT;
+                ide_set_irq(s);
+                break;
             case 0x03: { /* set transfer mode */
 		uint8_t val = s->nsector & 0x07;
 
@@ -2814,6 +2843,7 @@ static void ide_init2(IDEState *ide_state,
         s->irq = irq;
         s->sector_write_timer = qemu_new_timer(vm_clock,
                                                ide_sector_write_timer_cb, s);
+	s->write_cache = 0;
         ide_reset(s);
     }
 }
@@ -2842,6 +2872,7 @@ static void ide_save(QEMUFile* f, IDEState *s)
     if (s->identify_set) {
         qemu_put_buffer(f, (const uint8_t *)s->identify_data, 512);
     }
+    qemu_put_8s(f, &s->write_cache);
     qemu_put_8s(f, &s->feature);
     qemu_put_8s(f, &s->error);
     qemu_put_be32s(f, &s->nsector);
@@ -2863,13 +2894,15 @@ static void ide_save(QEMUFile* f, IDEState *s)
 }
 
 /* load per IDE drive data */
-static void ide_load(QEMUFile* f, IDEState *s)
+static void ide_load(QEMUFile* f, IDEState *s, int version_id)
 {
     s->mult_sectors=qemu_get_be32(f);
     s->identify_set=qemu_get_be32(f);
     if (s->identify_set) {
         qemu_get_buffer(f, (uint8_t *)s->identify_data, 512);
     }
+    if (version_id >= 2)
+	qemu_get_8s(f, &s->write_cache);
     qemu_get_8s(f, &s->feature);
     qemu_get_8s(f, &s->error);
     qemu_get_be32s(f, &s->nsector);
@@ -3269,7 +3302,7 @@ static int pci_ide_load(QEMUFile* f, void *opaque, int version_id)
     PCIIDEState *d = opaque;
     int ret, i;
 
-    if (version_id != 1)
+    if (version_id != 1 && version_id != 2)
         return -EINVAL;
     ret = pci_device_load(&d->dev, f);
     if (ret < 0)
@@ -3294,7 +3327,7 @@ static int pci_ide_load(QEMUFile* f, void *opaque, int version_id)
 
     /* per IDE drive data */
     for(i = 0; i < 4; i++) {
-        ide_load(f, &d->ide_if[i]);
+        ide_load(f, &d->ide_if[i], version_id);
     }
     return 0;
 }
@@ -3351,7 +3384,7 @@ void pci_piix3_ide_init(PCIBus *bus, BlockDriverState **hd_table, int devfn,
     ide_init_ioport(&d->ide_if[0], 0x1f0, 0x3f6);
     ide_init_ioport(&d->ide_if[2], 0x170, 0x376);
 
-    register_savevm("ide", 0, 1, pci_ide_save, pci_ide_load, d);
+    register_savevm("ide", 0, 2, pci_ide_save, pci_ide_load, d);
 }
 
 /* hd_table must contain 4 block drivers */
@@ -3390,7 +3423,7 @@ void pci_piix4_ide_init(PCIBus *bus, BlockDriverState **hd_table, int devfn,
     ide_init_ioport(&d->ide_if[0], 0x1f0, 0x3f6);
     ide_init_ioport(&d->ide_if[2], 0x170, 0x376);
 
-    register_savevm("ide", 0, 1, pci_ide_save, pci_ide_load, d);
+    register_savevm("ide", 0, 2, pci_ide_save, pci_ide_load, d);
 }
 
 /***********************************************************/
@@ -3824,7 +3857,7 @@ static int md_load(QEMUFile *f, void *opaque, int version_id)
     s->ide->cur_drive = &s->ide[(drive1_selected != 0)];
 
     for (i = 0; i < 2; i ++)
-        ide_load(f, &s->ide[i]);
+        ide_load(f, &s->ide[i], version_id);
 
     return 0;
 }
-- 
1.4.4.4

[Qemu-devel] [REPOST] [PATCH 0/3] IDE write cache

[Ian Jackson]

In these patches I provide control by the guest of the host buffer
cache, via the standard IDE commands for flush cache and write cache
enable/disable.

These must be applied after my IDE error checking patches because they
depend on the new non-void-returning bdrv_flush.

Ian.