Re: [Qemu-devel] [PATCH v1 1/1] hw/scsi: Report errors and sense to guests through scsi-block
On 7/6/19 2:08 AM, Paolo Bonzini wrote: > On 05/07/19 12:31, Shinichiro Kawasaki wrote: >>> This way there's generally no need to shoehorn ASC codes into errno. I >>> still have to test my changes, but I hope to send something within a >>> couple of days. >> >> Thanks for sharing your thoughts. Now I understand your idea. >> >> I'm awaiting your patch. In case you want me to create the patch based on >> your >> idea, please let me know. I can afford some time next week to work on it. > > I didn't have time to finish, but since I will be out for part of next > week, here is what I currently have (untested, somewhat uncompiled). Paolo, thanks for the patch! With a few simple compile error fixes, the patch series worked with my system as expected. Zoned storage devices are visible on the guest through scsi-block driver. Good. Here I list the compile error fixes for your reference: 1) changed return type of scsi_sense_buf_is_guest_recoverable() from int to bool in include/scsi/utils.h 2) replaced scsi_sense_to_errno() call in scsi_sense_buf_is_guest_recoverable() with scsi_sense_is_guest_recoverable() 3) removed unused "out:" label in block/iscsi.c -- Best Regards, Shin'ichiro Kawasaki
Re: [Qemu-devel] [PATCH v1 1/1] hw/scsi: Report errors and sense to guests through scsi-block
On 05/07/19 12:31, Shinichiro Kawasaki wrote: >> This way there's generally no need to shoehorn ASC codes into errno. I >> still have to test my changes, but I hope to send something within a >> couple of days. > > Thanks for sharing your thoughts. Now I understand your idea. > > I'm awaiting your patch. In case you want me to create the patch based on > your > idea, please let me know. I can afford some time next week to work on it. I didn't have time to finish, but since I will be out for part of next week, here is what I currently have (untested, somewhat uncompiled). It's a bugfix so it can be included after hard freeze. Thanks! Paolo >From fdccbd77192ca4450f6e8900bef392fa36242200 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 5 Jul 2019 19:07:17 +0200 Subject: [PATCH 0/5] *** SUBJECT HERE *** *** BLURB HERE *** Paolo Bonzini (4): scsi: explicitly list guest-recoverable sense codes scsi: add guest-recoverable ZBC errors iscsi: fix busy/timeout/task set full iscsi: base all handling of check condition on scsi_sense_to_errno Shinichiro Kawasaki (1): scsi-disk: pass sense correctly for guest-recoverable errors block/iscsi.c| 28 +++ hw/scsi/scsi-disk.c | 15 ++--- include/scsi/utils.h | 1 + scsi/utils.c | 53 +--- 4 files changed, 77 insertions(+), 20 deletions(-) -- 2.21.0 >From f8a4b9cd0cd878aacbdf4ad4e7a2451c87339dab Mon Sep 17 00:00:00 2001 From: Shinichiro Kawasaki Date: Tue, 2 Jul 2019 10:08:00 +0200 Subject: [PATCH 1/5] scsi-disk: pass sense correctly for guest-recoverable errors When an error was passed down to the guest because it was recoverable, the sense length was not copied from the SG_IO data. As a result, the guest saw the CHECK CONDITION status but not the sense data. Signed-off-by: Shinichiro Kawasaki Signed-off-by: Paolo Bonzini --- hw/scsi/scsi-disk.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index ed7295bfd7..5d3fb3c9d5 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); @@ -456,6 +458,7 @@ static bool scsi_handle_rw_error(SCSIDiskReq *r, int error, bool acct_failed) if (error == ECANCELED || error == EAGAIN || error == ENOTCONN || error == 0) { /* These errors are handled by guest. */ +sdc->update_sense(&r->req); scsi_req_complete(&r->req, *r->status); return true; } @@ -2894,6 +2897,12 @@ 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 +3068,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; -- 2.21.0 >From ab60dfcd4c2bbad79d2bdfae37a88b031917ec99 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 2 Jul 2019 10:23:20 +0200 Subject: [PATCH 2/5] scsi: explicitly list guest-recoverable sense codes It's not really possible to fit all sense codes into errno codes, especially in such a way that sense codes can be properly categorized as either. guest-recoverable and host-handled. Just create a new function that checks for guest recoverable sense, then scsi_sense_buf_to_errno only needs to be called for host handled sense codes. --- hw/scsi/scsi-disk.c | 5 ++--- include/scsi/utils.h | 1 + scsi/utils.c | 43 +++ 3 files changed, 46 insertions(+), 3 deletions(-) diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index 5d3fb3c9d5..8e95e3e38d 100644 --- a/hw/scsi/scsi-disk.c +++ b/hw/scsi/scsi-disk.c @@ -454,14 +454,13 @@ static bool scsi_handle_rw_error(SCSIDi
Re: [Qemu-devel] [PATCH v1 1/1] hw/scsi: Report errors and sense to guests through scsi-block
On 7/2/19 7:23 PM, Paolo Bonzini wrote: > On 02/07/19 08:44, Shinichiro Kawasaki wrote: >> On 7/1/19 8:56 PM, Paolo Bonzini wrote: >>> On 01/07/19 12:14, Shinichiro Kawasaki wrote: I observe four of them listed below in sense data, when I ran basic operations to the zoned storage from the guest via scsi-block. 21h 04h: UNALIGNED WRITE COMMAND 21h 05h: WRITE BOUNDARY VIOLATION 21h 06h: ATTEMPT TO READ INVALID DATA 55h 0Eh: INSUFFICIENT ZONE RESOURCES These ASCs can be reported for write or read commands due to unexpected zone status or write pointer status. Reporting these ASCs to the guest, the user applications can handle them to manage zone/write pointer status, or help the user application developers to understand the failure reason and fix bugs. I took a look in scsi_sense_to_errno() and learned that ASCs are grouped in errnos. To report the ASCs above to the guest, is it good to add them in EINVAL group defined in scsi_sense_to_errno()? The ASCs are reported with sense key ILLEGAL_REQUEST or DATA_PROTECT, then I think it fits in the function. >>> >>> The grouping by errno is historical and pretty much broken. It should >>> be possible to change it to return just a bool. >> >> The errno grouping of scsi_sense_to_errno() is used not only by scsi-disk but >> also by block/iscsi for error reporting. Can we avoid errno grouping for >> iscsi also? > > No, but we can do something like > > if (scsi_sense_buf_is_guest_recoverable(r->req.sense, > sizeof(r->req.sense))) { > /* These errors are handled by guest. */ > sdc->update_sense(&r->req); > scsi_req_complete(&r->req, *r->status); > return true; > } > error = scsi_sense_buf_to_errno(r->req.sense, sizeof(r->req.sense)); > > This way there's generally no need to shoehorn ASC codes into errno. I > still have to test my changes, but I hope to send something within a > couple of days. Thanks for sharing your thoughts. Now I understand your idea. I'm awaiting your patch. In case you want me to create the patch based on your idea, please let me know. I can afford some time next week to work on it. -- Best Regards, Shin'ichiro Kawasaki
Re: [Qemu-devel] [PATCH v1 1/1] hw/scsi: Report errors and sense to guests through scsi-block
On 02/07/19 08:44, Shinichiro Kawasaki wrote: > On 7/1/19 8:56 PM, Paolo Bonzini wrote: >> On 01/07/19 12:14, Shinichiro Kawasaki wrote: >>> I observe four of them listed below in sense data, >>> when I ran basic operations to the zoned storage from the guest via >>> scsi-block. >>> >>> 21h 04h: UNALIGNED WRITE COMMAND >>> 21h 05h: WRITE BOUNDARY VIOLATION >>> 21h 06h: ATTEMPT TO READ INVALID DATA >>> 55h 0Eh: INSUFFICIENT ZONE RESOURCES >>> >>> These ASCs can be reported for write or read commands due to unexpected zone >>> status or write pointer status. Reporting these ASCs to the guest, the user >>> applications can handle them to manage zone/write pointer status, or help >>> the >>> user application developers to understand the failure reason and fix bugs. >>> >>> I took a look in scsi_sense_to_errno() and learned that ASCs are grouped in >>> errnos. To report the ASCs above to the guest, is it good to add them in >>> EINVAL >>> group defined in scsi_sense_to_errno()? The ASCs are reported with sense key >>> ILLEGAL_REQUEST or DATA_PROTECT, then I think it fits in the function. >> >> The grouping by errno is historical and pretty much broken. It should >> be possible to change it to return just a bool. > > The errno grouping of scsi_sense_to_errno() is used not only by scsi-disk but > also by block/iscsi for error reporting. Can we avoid errno grouping for > iscsi also? No, but we can do something like if (scsi_sense_buf_is_guest_recoverable(r->req.sense, sizeof(r->req.sense))) { /* These errors are handled by guest. */ sdc->update_sense(&r->req); scsi_req_complete(&r->req, *r->status); return true; } error = scsi_sense_buf_to_errno(r->req.sense, sizeof(r->req.sense)); This way there's generally no need to shoehorn ASC codes into errno. I still have to test my changes, but I hope to send something within a couple of days. Paolo
Re: [Qemu-devel] [PATCH v1 1/1] hw/scsi: Report errors and sense to guests through scsi-block
On 7/1/19 8:56 PM, Paolo Bonzini wrote: > On 01/07/19 12:14, Shinichiro Kawasaki wrote: >> I observe four of them listed below in sense data, >> when I ran basic operations to the zoned storage from the guest via >> scsi-block. >> >> 21h 04h: UNALIGNED WRITE COMMAND >> 21h 05h: WRITE BOUNDARY VIOLATION >> 21h 06h: ATTEMPT TO READ INVALID DATA >> 55h 0Eh: INSUFFICIENT ZONE RESOURCES >> >> These ASCs can be reported for write or read commands due to unexpected zone >> status or write pointer status. Reporting these ASCs to the guest, the user >> applications can handle them to manage zone/write pointer status, or help the >> user application developers to understand the failure reason and fix bugs. >> >> I took a look in scsi_sense_to_errno() and learned that ASCs are grouped in >> errnos. To report the ASCs above to the guest, is it good to add them in >> EINVAL >> group defined in scsi_sense_to_errno()? The ASCs are reported with sense key >> ILLEGAL_REQUEST or DATA_PROTECT, then I think it fits in the function. > > The grouping by errno is historical and pretty much broken. It should > be possible to change it to return just a bool. The errno grouping of scsi_sense_to_errno() is used not only by scsi-disk but also by block/iscsi for error reporting. Can we avoid errno grouping for iscsi also? If the errno grouping is not valuable for iscsi, single error code (EIO?) can be reported for all iscsi failures. If the errno grouping is useful for iscsi, I can create a patch to avoid errno grouping only for scsi-disk, leaving scsi_sense_to_errno() for iscsi. -- Best Regards, Shin'ichiro Kawasaki
Re: [Qemu-devel] [PATCH v1 1/1] hw/scsi: Report errors and sense to guests through scsi-block
On 01/07/19 12:14, Shinichiro Kawasaki wrote: > I observe four of them listed below in sense data, > when I ran basic operations to the zoned storage from the guest via > scsi-block. > > 21h 04h: UNALIGNED WRITE COMMAND > 21h 05h: WRITE BOUNDARY VIOLATION > 21h 06h: ATTEMPT TO READ INVALID DATA > 55h 0Eh: INSUFFICIENT ZONE RESOURCES > > These ASCs can be reported for write or read commands due to unexpected zone > status or write pointer status. Reporting these ASCs to the guest, the user > applications can handle them to manage zone/write pointer status, or help the > user application developers to understand the failure reason and fix bugs. > > I took a look in scsi_sense_to_errno() and learned that ASCs are grouped in > errnos. To report the ASCs above to the guest, is it good to add them in > EINVAL > group defined in scsi_sense_to_errno()? The ASCs are reported with sense key > ILLEGAL_REQUEST or DATA_PROTECT, then I think it fits in the function. The grouping by errno is historical and pretty much broken. It should be possible to change it to return just a bool. Paolo
Re: [Qemu-devel] [PATCH v1 1/1] hw/scsi: Report errors and sense to guests through scsi-block
On 6/29/19 7:21 AM, Alistair Francis wrote: > On Fri, Jun 28, 2019 at 3:14 PM Paolo Bonzini wrote: >> >> On 28/06/19 23:57, Alistair Francis wrote: >>> >>> I tried to run my VM with option "-drive >>> ...,rerror=report,werror=report" as he noted, but the eternal loop >>> symptom still happens when host block-scsi device returns EIO. Then I >>> believe EIO should be added to the report target error list. >> >> What is the sense data he's seeing? EIO is a catch-all return value >> for scsi_sense_buf_to_errno so I'd rather be more specific. > > I'm not sure, he is CCed to this email so he should respond with more > information. Hi Paolo, thank you very much for your review and comments. (Alistair, thank you for your care for the patch post) The sense data I observe are related to 'zoned storage'. Now I'm trying to make zoned storage on host visible to qemu guests through scsi-block driver, to utilize qemu guest environment for zoned storage system development. 'zonedstroage.io' is the good reference for details of the zoned storage. http://zonedstorage.io/introduction/zoned-storage/ The zoned storage introduced "zone" and "write pointer" concepts, and SCSI spec documents were updated to define additional commands for zoned storage as well as Additional Sense Codes. The latest SPC-5 defines a number of ASCs that zoned SCSI storage devices return. I observe four of them listed below in sense data, when I ran basic operations to the zoned storage from the guest via scsi-block. 21h 04h: UNALIGNED WRITE COMMAND 21h 05h: WRITE BOUNDARY VIOLATION 21h 06h: ATTEMPT TO READ INVALID DATA 55h 0Eh: INSUFFICIENT ZONE RESOURCES These ASCs can be reported for write or read commands due to unexpected zone status or write pointer status. Reporting these ASCs to the guest, the user applications can handle them to manage zone/write pointer status, or help the user application developers to understand the failure reason and fix bugs. I took a look in scsi_sense_to_errno() and learned that ASCs are grouped in errnos. To report the ASCs above to the guest, is it good to add them in EINVAL group defined in scsi_sense_to_errno()? The ASCs are reported with sense key ILLEGAL_REQUEST or DATA_PROTECT, then I think it fits in the function. -- Best Regards, Shin'ichiro Kawasaki
Re: [Qemu-devel] [PATCH v1 1/1] hw/scsi: Report errors and sense to guests through scsi-block
On Fri, Jun 28, 2019 at 3:14 PM Paolo Bonzini wrote: > > On 28/06/19 23:57, Alistair Francis wrote: > > > > I tried to run my VM with option "-drive > > ...,rerror=report,werror=report" as he noted, but the eternal loop > > symptom still happens when host block-scsi device returns EIO. Then I > > believe EIO should be added to the report target error list. > > What is the sense data he's seeing? EIO is a catch-all return value > for scsi_sense_buf_to_errno so I'd rather be more specific. I'm not sure, he is CCed to this email so he should respond with more information. Alistair > > Thanks, > > Paolo
Re: [Qemu-devel] [PATCH v1 1/1] hw/scsi: Report errors and sense to guests through scsi-block
On 28/06/19 23:57, Alistair Francis wrote: > > I tried to run my VM with option "-drive > ...,rerror=report,werror=report" as he noted, but the eternal loop > symptom still happens when host block-scsi device returns EIO. Then I > believe EIO should be added to the report target error list. What is the sense data he's seeing? EIO is a catch-all return value for scsi_sense_buf_to_errno so I'd rather be more specific. Thanks, Paolo
Re: [Qemu-devel] [PATCH v1 1/1] hw/scsi: Report errors and sense to guests through scsi-block
On Thu, Jun 27, 2019 at 2:01 AM Paolo Bonzini wrote: > > On 27/06/19 00:46, Alistair Francis wrote: > > From: Shin'ichiro Kawasaki > > > > 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"? This is from Shin'ichiro Kawasaki: I tried to run my VM with option "-drive ...,rerror=report,werror=report" as he noted, but the eternal loop symptom still happens when host block-scsi device returns EIO. Then I believe EIO should be added to the report target error list. > > The update_sense part is okay too. Cool! Alistair > > 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 > > Signed-off-by: Alistair Francis > > --- > > 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; > > >
Re: [Qemu-devel] [PATCH v1 1/1] hw/scsi: Report errors and sense to guests through scsi-block
On 27/06/19 00:46, Alistair Francis wrote: > From: Shin'ichiro Kawasaki > > 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 > Signed-off-by: Alistair Francis > --- > 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; >
[Qemu-devel] [PATCH v1 1/1] hw/scsi: Report errors and sense to guests through scsi-block
From: Shin'ichiro Kawasaki 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. 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 Signed-off-by: Alistair Francis --- 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; -- 2.22.0