[Qemu-devel] [PATCH 0/4] rerror option for -drive

[Qemu-devel] [PATCH 0/4] rerror option for -drive

[Kevin Wolf]

We already have a werror option in -drive that controls the action to be taken
when write accesses to the image fail. A corresponding option for reads is
still missing and is added by this patch series.

The option is implemented on IDE and virtio-blk. I have left out SCSI for now
to avoid conflicts with kraxel's major restructuring work.

Kevin Wolf (4):
  Rename DriveInfo.onerror to on_write_error
  Introduce rerror option for drives
  ide: Implement rerror option
  virtio-blk: Implement rerror option

 hw/ide/core.c     |   54 +++++++++++++++++++++++++++++++++---------------
 hw/ide/internal.h |    1 +
 hw/scsi-disk.c    |    3 +-
 hw/virtio-blk.c   |   13 +++++++----
 qemu-config.c     |    3 ++
 sysemu.h          |    7 ++++-
 vl.c              |   58 ++++++++++++++++++++++++++++++++++++++--------------
 7 files changed, 98 insertions(+), 41 deletions(-)

[Qemu-devel] [PATCH 1/4] Rename DriveInfo.onerror to on_write_error

[Kevin Wolf]

Either rename variables and functions to refer to write errors (which is what
they actually do) or introduce a parameter to distinguish reads and writes.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/ide/core.c   |    2 +-
 hw/scsi-disk.c  |    3 ++-
 hw/virtio-blk.c |    2 +-
 sysemu.h        |    6 ++++--
 vl.c            |   11 ++++++++---
 5 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 7b1ff8f..49bbdcd 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -473,7 +473,7 @@ void ide_dma_error(IDEState *s)
 
 static int ide_handle_write_error(IDEState *s, int error, int op)
 {
-    BlockInterfaceErrorAction action = drive_get_onerror(s->bs);
+    BlockInterfaceErrorAction action = drive_get_on_error(s->bs, 0);
 
     if (action == BLOCK_ERR_IGNORE)
         return 0;
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index a92b62f..37f0cf3 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -220,7 +220,8 @@ static void scsi_read_data(SCSIDevice *d, uint32_t tag)
 
 static int scsi_handle_write_error(SCSIRequest *r, int error)
 {
-    BlockInterfaceErrorAction action = drive_get_onerror(r->dev->dinfo->bdrv);
+    BlockInterfaceErrorAction action =
+        drive_get_on_error(r->dev->dinfo->bdrv, 0);
 
     if (action == BLOCK_ERR_IGNORE)
         return 0;
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 42b766f..a93d20d 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -100,7 +100,7 @@ static void virtio_blk_req_complete(VirtIOBlockReq *req, int status)
 
 static int virtio_blk_handle_write_error(VirtIOBlockReq *req, int error)
 {
-    BlockInterfaceErrorAction action = drive_get_onerror(req->dev->bs);
+    BlockInterfaceErrorAction action = drive_get_on_error(req->dev->bs, 0);
     VirtIOBlock *s = req->dev;
 
     if (action == BLOCK_ERR_IGNORE)
diff --git a/sysemu.h b/sysemu.h
index b1887ef..522d9af 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -172,7 +172,7 @@ typedef struct DriveInfo {
     int bus;
     int unit;
     QemuOpts *opts;
-    BlockInterfaceErrorAction onerror;
+    BlockInterfaceErrorAction on_write_error;
     char serial[BLOCK_SERIAL_STRLEN + 1];
     QTAILQ_ENTRY(DriveInfo) next;
 } DriveInfo;
@@ -189,7 +189,9 @@ extern DriveInfo *drive_get_by_id(const char *id);
 extern int drive_get_max_bus(BlockInterfaceType type);
 extern void drive_uninit(DriveInfo *dinfo);
 extern const char *drive_get_serial(BlockDriverState *bdrv);
-extern BlockInterfaceErrorAction drive_get_onerror(BlockDriverState *bdrv);
+
+extern BlockInterfaceErrorAction drive_get_on_error(
+    BlockDriverState *bdrv, int is_read);
 
 BlockDriverState *qdev_init_bdrv(DeviceState *dev, BlockInterfaceType type);
 
diff --git a/vl.c b/vl.c
index ee43808..ecfecb3 100644
--- a/vl.c
+++ b/vl.c
@@ -1960,13 +1960,18 @@ const char *drive_get_serial(BlockDriverState *bdrv)
     return "\0";
 }
 
-BlockInterfaceErrorAction drive_get_onerror(BlockDriverState *bdrv)
+BlockInterfaceErrorAction drive_get_on_error(
+    BlockDriverState *bdrv, int is_read)
 {
     DriveInfo *dinfo;
 
+    if (is_read) {
+        return BLOCK_ERR_REPORT;
+    }
+
     QTAILQ_FOREACH(dinfo, &drives, next) {
         if (dinfo->bdrv == bdrv)
-            return dinfo->onerror;
+            return dinfo->on_write_error;
     }
 
     return BLOCK_ERR_STOP_ENOSPC;
@@ -2264,7 +2269,7 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque,
     dinfo->type = type;
     dinfo->bus = bus_id;
     dinfo->unit = unit_id;
-    dinfo->onerror = onerror;
+    dinfo->on_write_error = onerror;
     dinfo->opts = opts;
     if (serial)
         strncpy(dinfo->serial, serial, sizeof(serial));
-- 
1.6.2.5

[Qemu-devel] [PATCH 2/4] Introduce rerror option for drives

[Kevin Wolf]

rerror controls the action to be taken when an error occurs while accessing the
guest image file. It corresponds to werror which already controls the action
take for write errors.

This purely introduces parsing rerror command line option into the right
structures, real support for it in the device emulation is added in the
following patches.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qemu-config.c |    3 ++
 sysemu.h      |    1 +
 vl.c          |   59 ++++++++++++++++++++++++++++++++++++++------------------
 3 files changed, 44 insertions(+), 19 deletions(-)

diff --git a/qemu-config.c b/qemu-config.c
index 590fc05..92b5363 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -65,6 +65,9 @@ QemuOptsList qemu_drive_opts = {
             .name = "serial",
             .type = QEMU_OPT_STRING,
         },{
+            .name = "rerror",
+            .type = QEMU_OPT_STRING,
+        },{
             .name = "werror",
             .type = QEMU_OPT_STRING,
         },{
diff --git a/sysemu.h b/sysemu.h
index 522d9af..629d7e5 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -172,6 +172,7 @@ typedef struct DriveInfo {
     int bus;
     int unit;
     QemuOpts *opts;
+    BlockInterfaceErrorAction on_read_error;
     BlockInterfaceErrorAction on_write_error;
     char serial[BLOCK_SERIAL_STRLEN + 1];
     QTAILQ_ENTRY(DriveInfo) next;
diff --git a/vl.c b/vl.c
index ecfecb3..5b5dc60 100644
--- a/vl.c
+++ b/vl.c
@@ -1965,16 +1965,12 @@ BlockInterfaceErrorAction drive_get_on_error(
 {
     DriveInfo *dinfo;
 
-    if (is_read) {
-        return BLOCK_ERR_REPORT;
-    }
-
     QTAILQ_FOREACH(dinfo, &drives, next) {
         if (dinfo->bdrv == bdrv)
-            return dinfo->on_write_error;
+            return is_read ? dinfo->on_read_error : dinfo->on_write_error;
     }
 
-    return BLOCK_ERR_STOP_ENOSPC;
+    return is_read ? BLOCK_ERR_REPORT : BLOCK_ERR_STOP_ENOSPC;
 }
 
 static void bdrv_format_print(void *opaque, const char *name)
@@ -1990,6 +1986,23 @@ void drive_uninit(DriveInfo *dinfo)
     qemu_free(dinfo);
 }
 
+static int parse_block_error_action(const char *buf, int is_read)
+{
+    if (!strcmp(buf, "ignore")) {
+        return BLOCK_ERR_IGNORE;
+    } else if (!is_read && !strcmp(buf, "enospc")) {
+        return BLOCK_ERR_STOP_ENOSPC;
+    } else if (!strcmp(buf, "stop")) {
+        return BLOCK_ERR_STOP_ANY;
+    } else if (!strcmp(buf, "report")) {
+        return BLOCK_ERR_REPORT;
+    } else {
+        fprintf(stderr, "qemu: '%s' invalid %s error action\n",
+            buf, is_read ? "read" : "write");
+        return -1;
+    }
+}
+
 DriveInfo *drive_init(QemuOpts *opts, void *opaque,
                       int *fatal_error)
 {
@@ -2009,7 +2022,8 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque,
     int cache;
     int aio = 0;
     int ro = 0;
-    int bdrv_flags, onerror;
+    int bdrv_flags;
+    int on_read_error, on_write_error;
     const char *devaddr;
     DriveInfo *dinfo;
     int snapshot = 0;
@@ -2170,22 +2184,28 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque,
         }
     }
 
-    onerror = BLOCK_ERR_STOP_ENOSPC;
+    on_write_error = BLOCK_ERR_STOP_ENOSPC;
     if ((buf = qemu_opt_get(opts, "werror")) != NULL) {
         if (type != IF_IDE && type != IF_SCSI && type != IF_VIRTIO) {
             fprintf(stderr, "werror is no supported by this format\n");
             return NULL;
         }
-        if (!strcmp(buf, "ignore"))
-            onerror = BLOCK_ERR_IGNORE;
-        else if (!strcmp(buf, "enospc"))
-            onerror = BLOCK_ERR_STOP_ENOSPC;
-        else if (!strcmp(buf, "stop"))
-            onerror = BLOCK_ERR_STOP_ANY;
-        else if (!strcmp(buf, "report"))
-            onerror = BLOCK_ERR_REPORT;
-        else {
-            fprintf(stderr, "qemu: '%s' invalid write error action\n", buf);
+
+        on_write_error = parse_block_error_action(buf, 0);
+        if (on_write_error < 0) {
+            return NULL;
+        }
+    }
+
+    on_read_error = BLOCK_ERR_REPORT;
+    if ((buf = qemu_opt_get(opts, "rerror")) != NULL) {
+        if (1) {
+            fprintf(stderr, "rerror is no supported by this format\n");
+            return NULL;
+        }
+
+        on_read_error = parse_block_error_action(buf, 1);
+        if (on_read_error < 0) {
             return NULL;
         }
     }
@@ -2269,7 +2289,8 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque,
     dinfo->type = type;
     dinfo->bus = bus_id;
     dinfo->unit = unit_id;
-    dinfo->on_write_error = onerror;
+    dinfo->on_read_error = on_read_error;
+    dinfo->on_write_error = on_write_error;
     dinfo->opts = opts;
     if (serial)
         strncpy(dinfo->serial, serial, sizeof(serial));
-- 
1.6.2.5

[Qemu-devel] [PATCH 3/4] ide: Implement rerror option

[Kevin Wolf]

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/ide/core.c     |   54 ++++++++++++++++++++++++++++++++++++----------------
 hw/ide/internal.h |    1 +
 vl.c              |    2 +-
 3 files changed, 39 insertions(+), 18 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 49bbdcd..a5b00ae 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -61,8 +61,9 @@ static inline int media_is_cd(IDEState *s)
 }
 
 static void ide_dma_start(IDEState *s, BlockDriverCompletionFunc *dma_cb);
-static void ide_dma_restart(IDEState *s);
+static void ide_dma_restart(IDEState *s, int is_read);
 static void ide_atapi_cmd_read_dma_cb(void *opaque, int ret);
+static int ide_handle_rw_error(IDEState *s, int error, int op);
 
 static void padstr(char *str, const char *src, int len)
 {
@@ -407,8 +408,11 @@ static void ide_sector_read(IDEState *s)
             n = s->req_nb_sectors;
         ret = bdrv_read(s->bs, sector_num, s->io_buffer, n);
         if (ret != 0) {
-            ide_rw_error(s);
-            return;
+            if (ide_handle_rw_error(s, -ret,
+                BM_STATUS_PIO_RETRY | BM_STATUS_RETRY_READ))
+            {
+                return;
+            }
         }
         ide_transfer_start(s, s->io_buffer, 512 * n, ide_sector_read);
         ide_set_irq(s->bus);
@@ -471,9 +475,10 @@ void ide_dma_error(IDEState *s)
     ide_set_irq(s->bus);
 }
 
-static int ide_handle_write_error(IDEState *s, int error, int op)
+static int ide_handle_rw_error(IDEState *s, int error, int op)
 {
-    BlockInterfaceErrorAction action = drive_get_on_error(s->bs, 0);
+    int is_read = (op & BM_STATUS_RETRY_READ);
+    BlockInterfaceErrorAction action = drive_get_on_error(s->bs, is_read);
 
     if (action == BLOCK_ERR_IGNORE)
         return 0;
@@ -484,7 +489,7 @@ static int ide_handle_write_error(IDEState *s, int error, int op)
         s->bus->bmdma->status |= op;
         vm_stop(0);
     } else {
-        if (op == BM_STATUS_DMA_RETRY) {
+        if (op & BM_STATUS_DMA_RETRY) {
             dma_buf_commit(s, 0);
             ide_dma_error(s);
         } else {
@@ -551,9 +556,11 @@ static void ide_read_dma_cb(void *opaque, int ret)
     int64_t sector_num;
 
     if (ret < 0) {
-        dma_buf_commit(s, 1);
-	ide_dma_error(s);
-	return;
+        if (ide_handle_rw_error(s, -ret,
+            BM_STATUS_DMA_RETRY | BM_STATUS_RETRY_READ))
+        {
+            return;
+        }
     }
 
     n = s->io_buffer_size >> 9;
@@ -622,7 +629,7 @@ static void ide_sector_write(IDEState *s)
     ret = bdrv_write(s->bs, sector_num, s->io_buffer, n);
 
     if (ret != 0) {
-        if (ide_handle_write_error(s, -ret, BM_STATUS_PIO_RETRY))
+        if (ide_handle_rw_error(s, -ret, BM_STATUS_PIO_RETRY))
             return;
     }
 
@@ -658,16 +665,23 @@ static void ide_sector_write(IDEState *s)
 static void ide_dma_restart_bh(void *opaque)
 {
     BMDMAState *bm = opaque;
+    int is_read;
 
     qemu_bh_delete(bm->bh);
     bm->bh = NULL;
 
+    is_read = !!(bm->status & BM_STATUS_RETRY_READ);
+
     if (bm->status & BM_STATUS_DMA_RETRY) {
-        bm->status &= ~BM_STATUS_DMA_RETRY;
-        ide_dma_restart(bmdma_active_if(bm));
+        bm->status &= ~(BM_STATUS_DMA_RETRY | BM_STATUS_RETRY_READ);
+        ide_dma_restart(bmdma_active_if(bm), is_read);
     } else if (bm->status & BM_STATUS_PIO_RETRY) {
-        bm->status &= ~BM_STATUS_PIO_RETRY;
-        ide_sector_write(bmdma_active_if(bm));
+        bm->status &= ~(BM_STATUS_PIO_RETRY | BM_STATUS_RETRY_READ);
+        if (is_read) {
+            ide_sector_read(bmdma_active_if(bm));
+        } else {
+            ide_sector_write(bmdma_active_if(bm));
+        }
     }
 }
 
@@ -692,7 +706,7 @@ static void ide_write_dma_cb(void *opaque, int ret)
     int64_t sector_num;
 
     if (ret < 0) {
-        if (ide_handle_write_error(s, -ret,  BM_STATUS_DMA_RETRY))
+        if (ide_handle_rw_error(s, -ret,  BM_STATUS_DMA_RETRY))
             return;
     }
 
@@ -2715,7 +2729,7 @@ static void ide_dma_start(IDEState *s, BlockDriverCompletionFunc *dma_cb)
     }
 }
 
-static void ide_dma_restart(IDEState *s)
+static void ide_dma_restart(IDEState *s, int is_read)
 {
     BMDMAState *bm = s->bus->bmdma;
     ide_set_sector(s, bm->sector_num);
@@ -2723,7 +2737,13 @@ static void ide_dma_restart(IDEState *s)
     s->io_buffer_size = 0;
     s->nsector = bm->nsector;
     bm->cur_addr = bm->addr;
-    bm->dma_cb = ide_write_dma_cb;
+
+    if (is_read) {
+        bm->dma_cb = ide_read_dma_cb;
+    } else {
+        bm->dma_cb = ide_write_dma_cb;
+    }
+
     ide_dma_start(s, bm->dma_cb);
 }
 
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 567616e..f937daa 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -464,6 +464,7 @@ struct IDEDeviceInfo {
 #define BM_STATUS_INT    0x04
 #define BM_STATUS_DMA_RETRY  0x08
 #define BM_STATUS_PIO_RETRY  0x10
+#define BM_STATUS_RETRY_READ 0x20
 
 #define BM_CMD_START     0x01
 #define BM_CMD_READ      0x08
diff --git a/vl.c b/vl.c
index 5b5dc60..0136d16 100644
--- a/vl.c
+++ b/vl.c
@@ -2199,7 +2199,7 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque,
 
     on_read_error = BLOCK_ERR_REPORT;
     if ((buf = qemu_opt_get(opts, "rerror")) != NULL) {
-        if (1) {
+        if (type != IF_IDE) {
             fprintf(stderr, "rerror is no supported by this format\n");
             return NULL;
         }
-- 
1.6.2.5

[Qemu-devel] [PATCH 4/4] virtio-blk: Implement rerror option

[Kevin Wolf]

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/virtio-blk.c |   13 ++++++++-----
 vl.c            |    2 +-
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index a93d20d..a2f0639 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -98,9 +98,11 @@ static void virtio_blk_req_complete(VirtIOBlockReq *req, int status)
     qemu_free(req);
 }
 
-static int virtio_blk_handle_write_error(VirtIOBlockReq *req, int error)
+static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error,
+    int is_read)
 {
-    BlockInterfaceErrorAction action = drive_get_on_error(req->dev->bs, 0);
+    BlockInterfaceErrorAction action =
+        drive_get_on_error(req->dev->bs, is_read);
     VirtIOBlock *s = req->dev;
 
     if (action == BLOCK_ERR_IGNORE)
@@ -122,12 +124,13 @@ static void virtio_blk_rw_complete(void *opaque, int ret)
 {
     VirtIOBlockReq *req = opaque;
 
-    if (ret && (req->out->type & VIRTIO_BLK_T_OUT)) {
-        if (virtio_blk_handle_write_error(req, -ret))
+    if (ret) {
+        int is_read = !(req->out->type & VIRTIO_BLK_T_OUT);
+        if (virtio_blk_handle_rw_error(req, -ret, is_read))
             return;
     }
 
-    virtio_blk_req_complete(req, ret ? VIRTIO_BLK_S_IOERR : VIRTIO_BLK_S_OK);
+    virtio_blk_req_complete(req, VIRTIO_BLK_S_OK);
 }
 
 static void virtio_blk_flush_complete(void *opaque, int ret)
diff --git a/vl.c b/vl.c
index 0136d16..9f3962c 100644
--- a/vl.c
+++ b/vl.c
@@ -2199,7 +2199,7 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque,
 
     on_read_error = BLOCK_ERR_REPORT;
     if ((buf = qemu_opt_get(opts, "rerror")) != NULL) {
-        if (type != IF_IDE) {
+        if (type != IF_IDE && type != IF_VIRTIO) {
             fprintf(stderr, "rerror is no supported by this format\n");
             return NULL;
         }
-- 
1.6.2.5

Re: [Qemu-devel] [PATCH 1/4] Rename DriveInfo.onerror to on_write_error

[Gleb Natapov]

On Fri, Nov 27, 2009 at 01:25:36PM +0100, Kevin Wolf wrote:
> Either rename variables and functions to refer to write errors (which is what
> they actually do) or introduce a parameter to distinguish reads and writes.
> 
I prefer either to use two different functions or hide 0/1 parameter behind
a macro:
 #define drive_get_on_write_error(a) drive_get_on_error(a, 0);


> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  hw/ide/core.c   |    2 +-
>  hw/scsi-disk.c  |    3 ++-
>  hw/virtio-blk.c |    2 +-
>  sysemu.h        |    6 ++++--
>  vl.c            |   11 ++++++++---
>  5 files changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index 7b1ff8f..49bbdcd 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -473,7 +473,7 @@ void ide_dma_error(IDEState *s)
>  
>  static int ide_handle_write_error(IDEState *s, int error, int op)
>  {
> -    BlockInterfaceErrorAction action = drive_get_onerror(s->bs);
> +    BlockInterfaceErrorAction action = drive_get_on_error(s->bs, 0);
>  
>      if (action == BLOCK_ERR_IGNORE)
>          return 0;
> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
> index a92b62f..37f0cf3 100644
> --- a/hw/scsi-disk.c
> +++ b/hw/scsi-disk.c
> @@ -220,7 +220,8 @@ static void scsi_read_data(SCSIDevice *d, uint32_t tag)
>  
>  static int scsi_handle_write_error(SCSIRequest *r, int error)
>  {
> -    BlockInterfaceErrorAction action = drive_get_onerror(r->dev->dinfo->bdrv);
> +    BlockInterfaceErrorAction action =
> +        drive_get_on_error(r->dev->dinfo->bdrv, 0);
>  
>      if (action == BLOCK_ERR_IGNORE)
>          return 0;
> diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
> index 42b766f..a93d20d 100644
> --- a/hw/virtio-blk.c
> +++ b/hw/virtio-blk.c
> @@ -100,7 +100,7 @@ static void virtio_blk_req_complete(VirtIOBlockReq *req, int status)
>  
>  static int virtio_blk_handle_write_error(VirtIOBlockReq *req, int error)
>  {
> -    BlockInterfaceErrorAction action = drive_get_onerror(req->dev->bs);
> +    BlockInterfaceErrorAction action = drive_get_on_error(req->dev->bs, 0);
>      VirtIOBlock *s = req->dev;
>  
>      if (action == BLOCK_ERR_IGNORE)
> diff --git a/sysemu.h b/sysemu.h
> index b1887ef..522d9af 100644
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -172,7 +172,7 @@ typedef struct DriveInfo {
>      int bus;
>      int unit;
>      QemuOpts *opts;
> -    BlockInterfaceErrorAction onerror;
> +    BlockInterfaceErrorAction on_write_error;
>      char serial[BLOCK_SERIAL_STRLEN + 1];
>      QTAILQ_ENTRY(DriveInfo) next;
>  } DriveInfo;
> @@ -189,7 +189,9 @@ extern DriveInfo *drive_get_by_id(const char *id);
>  extern int drive_get_max_bus(BlockInterfaceType type);
>  extern void drive_uninit(DriveInfo *dinfo);
>  extern const char *drive_get_serial(BlockDriverState *bdrv);
> -extern BlockInterfaceErrorAction drive_get_onerror(BlockDriverState *bdrv);
> +
> +extern BlockInterfaceErrorAction drive_get_on_error(
> +    BlockDriverState *bdrv, int is_read);
>  
>  BlockDriverState *qdev_init_bdrv(DeviceState *dev, BlockInterfaceType type);
>  
> diff --git a/vl.c b/vl.c
> index ee43808..ecfecb3 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1960,13 +1960,18 @@ const char *drive_get_serial(BlockDriverState *bdrv)
>      return "\0";
>  }
>  
> -BlockInterfaceErrorAction drive_get_onerror(BlockDriverState *bdrv)
> +BlockInterfaceErrorAction drive_get_on_error(
> +    BlockDriverState *bdrv, int is_read)
>  {
>      DriveInfo *dinfo;
>  
> +    if (is_read) {
> +        return BLOCK_ERR_REPORT;
> +    }
> +
>      QTAILQ_FOREACH(dinfo, &drives, next) {
>          if (dinfo->bdrv == bdrv)
> -            return dinfo->onerror;
> +            return dinfo->on_write_error;
>      }
>  
>      return BLOCK_ERR_STOP_ENOSPC;
> @@ -2264,7 +2269,7 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque,
>      dinfo->type = type;
>      dinfo->bus = bus_id;
>      dinfo->unit = unit_id;
> -    dinfo->onerror = onerror;
> +    dinfo->on_write_error = onerror;
>      dinfo->opts = opts;
>      if (serial)
>          strncpy(dinfo->serial, serial, sizeof(serial));
> -- 
> 1.6.2.5
> 
> 

--
			Gleb.

Re: [Qemu-devel] [PATCH 1/4] Rename DriveInfo.onerror to on_write_error

[Anthony Liguori]

Gleb Natapov wrote:
> On Fri, Nov 27, 2009 at 01:25:36PM +0100, Kevin Wolf wrote:
>   
>> Either rename variables and functions to refer to write errors (which is what
>> they actually do) or introduce a parameter to distinguish reads and writes.
>>
>>     
> I prefer either to use two different functions or hide 0/1 parameter behind
> a macro:
>  #define drive_get_on_write_error(a) drive_get_on_error(a, 0);
>   

Or a static inline.  But honestly, having three globally scoped 
functions shouldn't be a problem.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 1/4] Rename DriveInfo.onerror to on_write_error

[Kevin Wolf]

Am 03.12.2009 20:55, schrieb Anthony Liguori:
> Gleb Natapov wrote:
>> On Fri, Nov 27, 2009 at 01:25:36PM +0100, Kevin Wolf wrote:
>>   
>>> Either rename variables and functions to refer to write errors (which is what
>>> they actually do) or introduce a parameter to distinguish reads and writes.
>>>
>>>     
>> I prefer either to use two different functions or hide 0/1 parameter behind
>> a macro:
>>  #define drive_get_on_write_error(a) drive_get_on_error(a, 0);
>>   
> 
> Or a static inline.  But honestly, having three globally scoped 
> functions shouldn't be a problem.

Well, in the end, the callers already have some kind of is_read and just
pass it on. Would we really gain anything if they needed to put a
five-line if block there instead of a one-line function call (or maybe
two lines if they need to extract the is_read flag from somewhere)?

So, I'm happy to add your static inline functions, but I wouldn't like
to use them...

Kevin