Re: [Qemu-devel] [PATCH v2] scsi-block: Handle error from host devices

[Fam Zheng]

On Wed, 04/04 09:47, Fam Zheng wrote:
> The callback of blk_aio_ioctl is not sensible to SCSI errors, so
> werror=stop doesn't work if ioctl returns 0 but the scsi status is
> error.
> 
> Peek at the sg_io_hdr_t fields and amend ret to fix that.
> 
> Signed-off-by: Fam Zheng <address@hidden>
> 
> ---
> 
> v2: Fix ->cb. [Daniel]
> ---
>  hw/scsi/scsi-disk.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> index f5ab767ab5..9cb08ff491 100644
> --- a/hw/scsi/scsi-disk.c
> +++ b/hw/scsi/scsi-disk.c
> @@ -2651,10 +2651,26 @@ typedef struct SCSIBlockReq {
>      /* Selected bytes of the original CDB, copied into our own CDB.  */
>      uint8_t cmd, cdb1, group_number;
>  
> +    BlockCompletionFunc *cb;
> +    void *cb_opaque;
> +
>      /* CDB passed to SG_IO.  */
>      uint8_t cdb[16];
>  } SCSIBlockReq;
>  
> +static void scsi_block_sgio_cb(void *opaque, int ret)
> +{
> +    SCSIBlockReq *req = opaque;
> +
> +    if (!ret &&
> +        (req->io_header.status ||
> +         req->io_header.host_status ||
> +         req->io_header.driver_status)) {
> +        ret = -EIO;
> +    }
> +    req->cb(req->cb_opaque, ret);
> +}
> +
>  static BlockAIOCB *scsi_block_do_sgio(SCSIBlockReq *req,
>                                        int64_t offset, QEMUIOVector *iov,
>                                        int direction,
> @@ -2734,7 +2750,10 @@ static BlockAIOCB *scsi_block_do_sgio(SCSIBlockReq 
> *req,
>      io_header->usr_ptr = r;
>      io_header->flags |= SG_FLAG_DIRECT_IO;
>  
> -    aiocb = blk_aio_ioctl(s->qdev.conf.blk, SG_IO, io_header, cb, opaque);
> +    req->cb = cb;
> +    req->cb_opaque = opaque;
> +    aiocb = blk_aio_ioctl(s->qdev.conf.blk, SG_IO, io_header,
> +                          scsi_block_sgio_cb, req);
>      assert(aiocb != NULL);
>      return aiocb;
>  }
> -- 
> 2.14.3
> 

Hmm, now I see scsi_block_dma_command sets 

    r->req.status = &r->io_header.status;

and scsi_disk_req_check_error already looks at the SCSI status code. Maybe
this patch is not necessary, I'll check.

Fam