On 27/06/19 00:46, Alistair Francis wrote: > From: Shin'ichiro Kawasaki <shinichiro.kawas...@wdc.com> > > When host block devices are bridged to a guest system through > virtio-scsi-pci and scsi-block driver, scsi_handle_rw_error() in > hw/scsi/scsi-disk.c checks the error number to judge which error to > report to the guests. EIO and EINVAL are not reported and ignored. Once > EIO or EINVAL happen, eternal wait of guest system happens. This problem > was observed with zoned block devices on the host system attached to the > guest via virtio-scsi-pci. To avoid the eternal wait, add EIO and EINVAL > to the list of error numbers to report to the guest.
This is certainly correct for EINVAL, I am not sure about EIO. Did you run the VM with "-drive ...,rerror=report,werror=report"? The update_sense part is okay too. Paolo > On top of this, it is required to report SCSI sense data to the guest > so that the guest can handle the error correctly. However, > scsi_handle_rw_error() does not passthrough sense data that host > scsi-block device reported. Instead, it newly generates fixed sense > data only for certain error numbers. This is inflexible to support new > error codes to report to guest. To avoid this inflexiblity, pass the SCSI > sense data that the host scsi-block device reported as is. To be more > precise, set valid sense_len in the SCSIDiskReq referring sb_len_wr that > host SCSI device SG_IO ioctl reported. Add update_sense callback to > SCSIDiskClass to refer the SG_IO ioctl result only when scsi-block device > is targeted. > > Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawas...@wdc.com> > Signed-off-by: Alistair Francis <alistair.fran...@wdc.com> > --- > hw/scsi/scsi-disk.c | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c > index ed7295bfd7..6801e3a0d0 100644 > --- a/hw/scsi/scsi-disk.c > +++ b/hw/scsi/scsi-disk.c > @@ -62,6 +62,7 @@ typedef struct SCSIDiskClass { > DMAIOFunc *dma_readv; > DMAIOFunc *dma_writev; > bool (*need_fua_emulation)(SCSICommand *cmd); > + void (*update_sense)(SCSIRequest *r); > } SCSIDiskClass; > > typedef struct SCSIDiskReq { > @@ -438,6 +439,7 @@ static bool scsi_handle_rw_error(SCSIDiskReq *r, int > error, bool acct_failed) > { > bool is_read = (r->req.cmd.mode == SCSI_XFER_FROM_DEV); > SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev); > + SCSIDiskClass *sdc = (SCSIDiskClass *) object_get_class(OBJECT(s)); > BlockErrorAction action = blk_get_error_action(s->qdev.conf.blk, > is_read, error); > > @@ -452,9 +454,12 @@ static bool scsi_handle_rw_error(SCSIDiskReq *r, int > error, bool acct_failed) > * pause the host. > */ > assert(r->status && *r->status); > + if (sdc->update_sense) { > + sdc->update_sense(&r->req); > + } > error = scsi_sense_buf_to_errno(r->req.sense, > sizeof(r->req.sense)); > if (error == ECANCELED || error == EAGAIN || error == ENOTCONN || > - error == 0) { > + error == EIO || error == EINVAL || error == 0) { > /* These errors are handled by guest. */ > scsi_req_complete(&r->req, *r->status); > return true; > @@ -2894,6 +2899,13 @@ static int scsi_block_parse_cdb(SCSIDevice *d, > SCSICommand *cmd, > } > } > > +static void scsi_block_update_sense(SCSIRequest *req) > +{ > + SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req); > + SCSIBlockReq *br = DO_UPCAST(SCSIBlockReq, req, r); > + r->req.sense_len = MIN(br->io_header.sb_len_wr, sizeof(r->req.sense)); > +} > + > #endif > > static > @@ -3059,6 +3071,7 @@ static void scsi_block_class_initfn(ObjectClass *klass, > void *data) > sc->parse_cdb = scsi_block_parse_cdb; > sdc->dma_readv = scsi_block_dma_readv; > sdc->dma_writev = scsi_block_dma_writev; > + sdc->update_sense = scsi_block_update_sense; > sdc->need_fua_emulation = scsi_block_no_fua; > dc->desc = "SCSI block device passthrough"; > dc->props = scsi_block_properties; >