[Xen-devel] [PATCH] actually check for errors in bdrv_flush et al

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

bdrv_flush is declared to return void, but this is wrong because it
means that the implementations have nowhere to report their errors.
Indeed, the implementations generally ignore errors.

This patch corrects this by making it return int (implicitly, either 0
or -errno, as for other similar functions).  All of the
implementations and callers are adjusted too.

Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>

diff -r b5fea3aeb04b tools/ioemu/block-qcow.c
--- a/tools/ioemu/block-qcow.c  Fri Mar 28 14:12:33 2008 +0000
+++ b/tools/ioemu/block-qcow.c  Fri Mar 28 16:58:09 2008 +0000
@@ -934,10 +934,10 @@ static int qcow_write_compressed(BlockDr
     return 0;
 }
 
-static void qcow_flush(BlockDriverState *bs)
-{
-    BDRVQcowState *s = bs->opaque;
-    bdrv_flush(s->hd);
+static int qcow_flush(BlockDriverState *bs)
+{
+    BDRVQcowState *s = bs->opaque;
+    return bdrv_flush(s->hd);
 }
 
 static int qcow_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
diff -r b5fea3aeb04b tools/ioemu/block-qcow2.c
--- a/tools/ioemu/block-qcow2.c Fri Mar 28 14:12:33 2008 +0000
+++ b/tools/ioemu/block-qcow2.c Fri Mar 28 16:58:10 2008 +0000
@@ -1235,10 +1235,10 @@ static int qcow_write_compressed(BlockDr
     return 0;
 }
 
-static void qcow_flush(BlockDriverState *bs)
-{
-    BDRVQcowState *s = bs->opaque;
-    bdrv_flush(s->hd);
+static int qcow_flush(BlockDriverState *bs)
+{
+    BDRVQcowState *s = bs->opaque;
+    return bdrv_flush(s->hd);
 }
 
 static int qcow_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
diff -r b5fea3aeb04b tools/ioemu/block-raw.c
--- a/tools/ioemu/block-raw.c   Fri Mar 28 14:12:33 2008 +0000
+++ b/tools/ioemu/block-raw.c   Fri Mar 28 16:58:12 2008 +0000
@@ -615,10 +615,12 @@ static int raw_create(const char *filena
     return 0;
 }
 
-static void raw_flush(BlockDriverState *bs)
-{
-    BDRVRawState *s = bs->opaque;
-    fsync(s->fd);
+static int raw_flush(BlockDriverState *bs)
+{
+    BDRVRawState *s = bs->opaque;
+    if (fsync(s->fd))
+        return errno;
+    return 0;
 }
 
 BlockDriver bdrv_raw = {
diff -r b5fea3aeb04b tools/ioemu/block-vmdk.c
--- a/tools/ioemu/block-vmdk.c  Fri Mar 28 14:12:33 2008 +0000
+++ b/tools/ioemu/block-vmdk.c  Fri Mar 28 16:58:13 2008 +0000
@@ -734,10 +734,10 @@ static void vmdk_close(BlockDriverState 
     vmdk_parent_close(s->hd);
 }
 
-static void vmdk_flush(BlockDriverState *bs)
-{
-    BDRVVmdkState *s = bs->opaque;
-    bdrv_flush(s->hd);
+static int vmdk_flush(BlockDriverState *bs)
+{
+    BDRVVmdkState *s = bs->opaque;
+    return bdrv_flush(s->hd);
 }
 
 BlockDriver bdrv_vmdk = {
diff -r b5fea3aeb04b tools/ioemu/block.c
--- a/tools/ioemu/block.c       Fri Mar 28 14:12:33 2008 +0000
+++ b/tools/ioemu/block.c       Fri Mar 28 17:00:28 2008 +0000
@@ -889,12 +889,14 @@ const char *bdrv_get_device_name(BlockDr
     return bs->device_name;
 }
 
-void bdrv_flush(BlockDriverState *bs)
-{
-    if (bs->drv->bdrv_flush)
-        bs->drv->bdrv_flush(bs);
-    if (bs->backing_hd)
-        bdrv_flush(bs->backing_hd);
+int bdrv_flush(BlockDriverState *bs)
+{
+    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;
 }
 
 void bdrv_info(void)
@@ -1232,8 +1234,9 @@ static BlockDriverAIOCB *bdrv_aio_flush_
 static BlockDriverAIOCB *bdrv_aio_flush_em(BlockDriverState *bs,
         BlockDriverCompletionFunc *cb, void *opaque)
 {
-    bdrv_flush(bs);
-    cb(opaque, 0);
+    int ret;
+    ret = bdrv_flush(bs);
+    cb(opaque, ret);
     return NULL;
 }
 
diff -r b5fea3aeb04b tools/ioemu/block_int.h
--- a/tools/ioemu/block_int.h   Fri Mar 28 14:12:33 2008 +0000
+++ b/tools/ioemu/block_int.h   Fri Mar 28 16:58:15 2008 +0000
@@ -36,7 +36,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 -r b5fea3aeb04b tools/ioemu/hw/ide.c
--- a/tools/ioemu/hw/ide.c      Fri Mar 28 14:12:33 2008 +0000
+++ b/tools/ioemu/hw/ide.c      Fri Mar 28 16:58:15 2008 +0000
@@ -1786,6 +1786,7 @@ static void ide_ioport_write(void *opaqu
     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);
diff -r b5fea3aeb04b tools/ioemu/hw/scsi-disk.c
--- a/tools/ioemu/hw/scsi-disk.c        Fri Mar 28 14:12:33 2008 +0000
+++ b/tools/ioemu/hw/scsi-disk.c        Fri Mar 28 16:59:42 2008 +0000
@@ -291,6 +291,7 @@ int32_t scsi_send_command(SCSIDevice *s,
     uint8_t command;
     uint8_t *outbuf;
     SCSIRequest *r;
+    int ret;
 
     command = buf[0];
     r = scsi_find_request(s, tag);
@@ -496,7 +497,12 @@ int32_t scsi_send_command(SCSIDevice *s,
         break;
     case 0x35:
         DPRINTF("Syncronise 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:
         {
diff -r b5fea3aeb04b tools/ioemu/vl.h
--- a/tools/ioemu/vl.h  Fri Mar 28 14:12:33 2008 +0000
+++ b/tools/ioemu/vl.h  Fri Mar 28 16:58:15 2008 +0000
@@ -664,7 +664,7 @@ void qemu_aio_wait_end(void);
 void qemu_aio_wait_end(void);
 
 /* Ensure contents are flushed to disk.  */
-void bdrv_flush(BlockDriverState *bs);
+int bdrv_flush(BlockDriverState *bs);
 
 #define BDRV_TYPE_HD     0
 #define BDRV_TYPE_CDROM  1

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel