mbox series

[0/7] scsi: scsi-disk corrupts data

Message ID 20201116184041.60465-1-hare@suse.de (mailing list archive)
Headers show
Series scsi: scsi-disk corrupts data | expand

Message

Hannes Reinecke Nov. 16, 2020, 6:40 p.m. UTC
Hi all,

a customer of ours reported repeated data corruption in the guest following a command abort.
After lengthy debugging we found that scsi-disk (and scsi-generic, for that matter) ignores
the host_status field from SG_IO once a command is aborted. If the command is aborted, SG_IO
will return with a SCSI status 'GOOD', and host_status 'DID_TIME_OUT'. scsi-disk will now
ignore the DID_TIME_OUT setting, and just report the SCSI status back to the guest.
The guest will then assume everything is okay and not retry the command, leading to the data
corruption.

This patchset moves the (linux only) SG_ERR host_status codes to generic code as SCSI_HOST
values, and adds a host_status field to SCSIRequest. With that some drivers like virtio_scsi
can interpret the host_status code and map it onto it driver-specific status.
This status is then visible to the guest, which then is able to take appropriate action.

As usual, comments and reviews are welcome.

Hannes Reinecke (6):
  scsi-disk: Add sg_io callback to evaluate status
  scsi: drop 'result' argument from command_complete callback
  scsi: Rename linux-specific SG_ERR codes to generic SCSI_HOST error
    codes
  scsi: Add mapping for generic SCSI_HOST status to sense codes
  scsi: split sg_io_sense_from_errno() in two functions
  scsi: move host_status handling into SCSI drivers

Paolo Bonzini (1):
  scsi-disk: convert more errno values back to SCSI statuses

 hw/scsi/esp-pci.c      |   5 +--
 hw/scsi/esp.c          |  17 +++++--
 hw/scsi/lsi53c895a.c   |  17 +++++--
 hw/scsi/megasas.c      |  15 +++++--
 hw/scsi/mptsas.c       |  14 +++++-
 hw/scsi/scsi-bus.c     |   2 +-
 hw/scsi/scsi-disk.c    |  75 ++++++++++++++++++++-----------
 hw/scsi/scsi-generic.c |  21 ++++++---
 hw/scsi/spapr_vscsi.c  |  20 ++++++---
 hw/scsi/virtio-scsi.c  |  44 ++++++++++++++++--
 hw/scsi/vmw_pvscsi.c   |  29 +++++++++++-
 hw/usb/dev-storage.c   |   6 +--
 hw/usb/dev-uas.c       |   7 ++-
 include/hw/scsi/esp.h  |   2 +-
 include/hw/scsi/scsi.h |   5 ++-
 include/scsi/utils.h   |  29 +++++++-----
 scsi/qemu-pr-helper.c  |  14 ++++--
 scsi/utils.c           | 119 ++++++++++++++++++++++++++++++++++++-------------
 18 files changed, 328 insertions(+), 113 deletions(-)

Comments

Paolo Bonzini Dec. 17, 2020, 10:07 a.m. UTC | #1
On 16/11/20 19:40, Hannes Reinecke wrote:
> Hi all,
> 
> a customer of ours reported repeated data corruption in the guest following a command abort.
> After lengthy debugging we found that scsi-disk (and scsi-generic, for that matter) ignores
> the host_status field from SG_IO once a command is aborted. If the command is aborted, SG_IO
> will return with a SCSI status 'GOOD', and host_status 'DID_TIME_OUT'. scsi-disk will now
> ignore the DID_TIME_OUT setting, and just report the SCSI status back to the guest.
> The guest will then assume everything is okay and not retry the command, leading to the data
> corruption.
> 
> This patchset moves the (linux only) SG_ERR host_status codes to generic code as SCSI_HOST
> values, and adds a host_status field to SCSIRequest. With that some drivers like virtio_scsi
> can interpret the host_status code and map it onto it driver-specific status.
> This status is then visible to the guest, which then is able to take appropriate action.
> 
> As usual, comments and reviews are welcome.
> 
> Hannes Reinecke (6):
>    scsi-disk: Add sg_io callback to evaluate status
>    scsi: drop 'result' argument from command_complete callback
>    scsi: Rename linux-specific SG_ERR codes to generic SCSI_HOST error
>      codes
>    scsi: Add mapping for generic SCSI_HOST status to sense codes
>    scsi: split sg_io_sense_from_errno() in two functions
>    scsi: move host_status handling into SCSI drivers
> 
> Paolo Bonzini (1):
>    scsi-disk: convert more errno values back to SCSI statuses
> 
>   hw/scsi/esp-pci.c      |   5 +--
>   hw/scsi/esp.c          |  17 +++++--
>   hw/scsi/lsi53c895a.c   |  17 +++++--
>   hw/scsi/megasas.c      |  15 +++++--
>   hw/scsi/mptsas.c       |  14 +++++-
>   hw/scsi/scsi-bus.c     |   2 +-
>   hw/scsi/scsi-disk.c    |  75 ++++++++++++++++++++-----------
>   hw/scsi/scsi-generic.c |  21 ++++++---
>   hw/scsi/spapr_vscsi.c  |  20 ++++++---
>   hw/scsi/virtio-scsi.c  |  44 ++++++++++++++++--
>   hw/scsi/vmw_pvscsi.c   |  29 +++++++++++-
>   hw/usb/dev-storage.c   |   6 +--
>   hw/usb/dev-uas.c       |   7 ++-
>   include/hw/scsi/esp.h  |   2 +-
>   include/hw/scsi/scsi.h |   5 ++-
>   include/scsi/utils.h   |  29 +++++++-----
>   scsi/qemu-pr-helper.c  |  14 ++++--
>   scsi/utils.c           | 119 ++++++++++++++++++++++++++++++++++++-------------
>   18 files changed, 328 insertions(+), 113 deletions(-)
> 

Queued, thanks.

Paolo