[2/2] QMP: Add 'reason' member to the BLOCK_IO_ERROR event

It's a parsable errno string representation, this is needed
because some management tools want to base their action on
the error cause.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 QMP/qmp-events.txt |    4 +++-
 block.c            |    8 +++++---
 block.h            |    2 +-
 hw/ide/core.c      |    6 +++---
 hw/scsi-disk.c     |    6 +++---
 hw/virtio-blk.c    |    6 +++---
 6 files changed, 18 insertions(+), 14 deletions(-)

On 04/28/2010 03:32 PM, Luiz Capitulino wrote:
> It's a parsable errno string representation, this is needed
> because some management tools want to base their action on
> the error cause.
>
> Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>
>    

Does anyone differentiate beyond ENOSPC and EIO?

Regards,

Anthony Liguori

> ---
>   QMP/qmp-events.txt |    4 +++-
>   block.c            |    8 +++++---
>   block.h            |    2 +-
>   hw/ide/core.c      |    6 +++---
>   hw/scsi-disk.c     |    6 +++---
>   hw/virtio-blk.c    |    6 +++---
>   6 files changed, 18 insertions(+), 14 deletions(-)
>
> diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
> index 01ec85f..47097fd 100644
> --- a/QMP/qmp-events.txt
> +++ b/QMP/qmp-events.txt
> @@ -14,13 +14,15 @@ Data:
>       "ignore": error has been ignored
>       "report": error has been reported to the device
>       "stop": error caused VM to be stopped
> +- "reason": errno string representation (json-string)
>
>   Example:
>
>   { "event": "BLOCK_IO_ERROR",
>       "data": { "device": "ide0-hd1",
>                 "operation": "write",
> -              "action": "stop" },
> +              "action": "stop",
> +              "reason": "EINTR" },
>       "timestamp": { "seconds": 1265044230, "microseconds": 450486 } }
>
>   Note: If action is "stop", a STOP event will eventually follow the
> diff --git a/block.c b/block.c
> index 7974215..1179b1c 100644
> --- a/block.c
> +++ b/block.c
> @@ -1236,7 +1236,7 @@ int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
>   }
>
>   void bdrv_mon_event(const BlockDriverState *bdrv,
> -                    BlockMonEventAction action, int is_read)
> +                    BlockMonEventAction action, int error, int is_read)
>   {
>       QObject *data;
>       const char *action_str;
> @@ -1255,10 +1255,12 @@ void bdrv_mon_event(const BlockDriverState *bdrv,
>           abort();
>       }
>
> -    data = qobject_from_jsonf("{ 'device': %s, 'action': %s, 'operation': %s }",
> +    data = qobject_from_jsonf("{ 'device': %s, 'action': %s, "
> +                              "'operation': %s, 'reason': %s }",
>                                 bdrv->device_name,
>                                 action_str,
> -                              is_read ? "read" : "write");
> +                              is_read ? "read" : "write",
> +                              get_errno_name(errno));
>       monitor_protocol_event(QEVENT_BLOCK_IO_ERROR, data);
>
>       qobject_decref(data);
> diff --git a/block.h b/block.h
> index 05ad572..9696081 100644
> --- a/block.h
> +++ b/block.h
> @@ -45,7 +45,7 @@ typedef enum {
>   } BlockMonEventAction;
>
>   void bdrv_mon_event(const BlockDriverState *bdrv,
> -                    BlockMonEventAction action, int is_read);
> +                    BlockMonEventAction action, int error, int is_read);
>   void bdrv_info_print(Monitor *mon, const QObject *data);
>   void bdrv_info(Monitor *mon, QObject **ret_data);
>   void bdrv_stats_print(Monitor *mon, const QObject *data);
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index 0757528..1ce0cdd 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -484,7 +484,7 @@ static int ide_handle_rw_error(IDEState *s, int error, int op)
>       BlockInterfaceErrorAction action = drive_get_on_error(s->bs, is_read);
>
>       if (action == BLOCK_ERR_IGNORE) {
> -        bdrv_mon_event(s->bs, BDRV_ACTION_IGNORE, is_read);
> +        bdrv_mon_event(s->bs, BDRV_ACTION_IGNORE, error, is_read);
>           return 0;
>       }
>
> @@ -492,7 +492,7 @@ static int ide_handle_rw_error(IDEState *s, int error, int op)
>               || action == BLOCK_ERR_STOP_ANY) {
>           s->bus->bmdma->unit = s->unit;
>           s->bus->bmdma->status |= op;
> -        bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read);
> +        bdrv_mon_event(s->bs, BDRV_ACTION_STOP, error, is_read);
>           vm_stop(0);
>       } else {
>           if (op&  BM_STATUS_DMA_RETRY) {
> @@ -501,7 +501,7 @@ static int ide_handle_rw_error(IDEState *s, int error, int op)
>           } else {
>               ide_rw_error(s);
>           }
> -        bdrv_mon_event(s->bs, BDRV_ACTION_REPORT, is_read);
> +        bdrv_mon_event(s->bs, BDRV_ACTION_REPORT, error, is_read);
>       }
>
>       return 1;
> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
> index 77cb1da..1df9552 100644
> --- a/hw/scsi-disk.c
> +++ b/hw/scsi-disk.c
> @@ -182,19 +182,19 @@ static int scsi_handle_write_error(SCSIDiskReq *r, int error)
>       BlockInterfaceErrorAction action = drive_get_on_error(s->bs, 0);
>
>       if (action == BLOCK_ERR_IGNORE) {
> -        bdrv_mon_event(s->bs, BDRV_ACTION_IGNORE, 0);
> +        bdrv_mon_event(s->bs, BDRV_ACTION_IGNORE, error, 0);
>           return 0;
>       }
>
>       if ((error == ENOSPC&&  action == BLOCK_ERR_STOP_ENOSPC)
>               || action == BLOCK_ERR_STOP_ANY) {
>           r->status |= SCSI_REQ_STATUS_RETRY;
> -        bdrv_mon_event(s->bs, BDRV_ACTION_STOP, 0);
> +        bdrv_mon_event(s->bs, BDRV_ACTION_STOP, error, 0);
>           vm_stop(0);
>       } else {
>           scsi_command_complete(r, CHECK_CONDITION,
>                   HARDWARE_ERROR);
> -        bdrv_mon_event(s->bs, BDRV_ACTION_REPORT, 0);
> +        bdrv_mon_event(s->bs, BDRV_ACTION_REPORT, error, 0);
>       }
>
>       return 1;
> diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
> index b05d15e..b0c504e 100644
> --- a/hw/virtio-blk.c
> +++ b/hw/virtio-blk.c
> @@ -65,7 +65,7 @@ static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error,
>       VirtIOBlock *s = req->dev;
>
>       if (action == BLOCK_ERR_IGNORE) {
> -        bdrv_mon_event(s->bs, BDRV_ACTION_IGNORE, is_read);
> +        bdrv_mon_event(s->bs, BDRV_ACTION_IGNORE, error, is_read);
>           return 0;
>       }
>
> @@ -73,11 +73,11 @@ static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error,
>               || action == BLOCK_ERR_STOP_ANY) {
>           req->next = s->rq;
>           s->rq = req;
> -        bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read);
> +        bdrv_mon_event(s->bs, BDRV_ACTION_STOP, error, is_read);
>           vm_stop(0);
>       } else {
>           virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR);
> -        bdrv_mon_event(s->bs, BDRV_ACTION_REPORT, is_read);
> +        bdrv_mon_event(s->bs, BDRV_ACTION_REPORT, error, is_read);
>       }
>
>       return 1;
>

On Wed, 28 Apr 2010 18:24:29 -0500
Anthony Liguori <anthony@codemonkey.ws> wrote:

> On 04/28/2010 03:32 PM, Luiz Capitulino wrote:
> > It's a parsable errno string representation, this is needed
> > because some management tools want to base their action on
> > the error cause.
> >
> > Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>
> >    
> 
> Does anyone differentiate beyond ENOSPC and EIO?

 For this specific case, I guess not.

 But this general solution is also needed in other places like
savevm, loadvm and delvm I was working (and sent) an RFC last week,
also I think other events and errors might need this in the near future.

 IOW, I expect this to be a QMP idiom.

On 04/29/2010 12:30 PM, Luiz Capitulino wrote:
> On Wed, 28 Apr 2010 18:24:29 -0500
> Anthony Liguori<anthony@codemonkey.ws>  wrote:
>
>    
>> On 04/28/2010 03:32 PM, Luiz Capitulino wrote:
>>      
>>> It's a parsable errno string representation, this is needed
>>> because some management tools want to base their action on
>>> the error cause.
>>>
>>> Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>
>>>
>>>        
>> Does anyone differentiate beyond ENOSPC and EIO?
>>      
>   For this specific case, I guess not.
>
>   But this general solution is also needed in other places like
> savevm, loadvm and delvm I was working (and sent) an RFC last week,
> also I think other events and errors might need this in the near future.
>
>   IOW, I expect this to be a QMP idiom.
>    

I think that's a bit of a crutch.  What's the point of QError if it's 
just encapsulating errno's?

Regards,

Anthony Liguori