From: Paolo Bonzini <pbonzini@redhat.com>
To: Alistair Francis <alistair.francis@wdc.com>, qemu-devel@nongnu.org
Cc: fam@euphon.net, shinichiro.kawasaki@wdc.com, alistair23@gmail.com
Subject: Re: [Qemu-devel] [PATCH v1 1/1] hw/scsi: Report errors and sense to guests through scsi-block
Date: Thu, 27 Jun 2019 11:01:09 +0200 [thread overview]
Message-ID: <1bd3ffcd-3f91-ecb9-2315-da7125f1dcdd@redhat.com> (raw)
In-Reply-To: <97104495f5c945d25315aff1bd618e1a7bacf34c.1561589072.git.alistair.francis@wdc.com>
On 27/06/19 00:46, Alistair Francis wrote:
> From: Shin'ichiro Kawasaki <shinichiro.kawasaki@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.kawasaki@wdc.com>
> Signed-off-by: Alistair Francis <alistair.francis@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;
>
next prev parent reply other threads:[~2019-06-27 9:03 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-26 22:46 [Qemu-devel] [PATCH v1 1/1] hw/scsi: Report errors and sense to guests through scsi-block Alistair Francis
2019-06-27 9:01 ` Paolo Bonzini [this message]
2019-06-28 21:57 ` Alistair Francis
2019-06-28 22:14 ` Paolo Bonzini
2019-06-28 22:18 ` Alistair Francis
2019-07-01 10:14 ` Shinichiro Kawasaki
2019-07-01 11:56 ` Paolo Bonzini
2019-07-02 6:44 ` Shinichiro Kawasaki
2019-07-02 10:22 ` Paolo Bonzini
2019-07-05 10:31 ` Shinichiro Kawasaki
2019-07-05 17:08 ` Paolo Bonzini
2019-07-09 8:27 ` Shinichiro Kawasaki
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1bd3ffcd-3f91-ecb9-2315-da7125f1dcdd@redhat.com \
--to=pbonzini@redhat.com \
--cc=alistair.francis@wdc.com \
--cc=alistair23@gmail.com \
--cc=fam@euphon.net \
--cc=qemu-devel@nongnu.org \
--cc=shinichiro.kawasaki@wdc.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).