[1/4] Rename DriveInfo.onerror to on_write_error

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(-)

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.

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

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