Re: [PATCH] scsi-disk: convert more errno values back to SCSI statuses

[Paolo Bonzini]

On 12/11/20 16:04, Hannes Reinecke wrote:
> On 11/12/20 10:52 AM, Paolo Bonzini wrote:
> Well, ironically I'm currently debugging a customer escalation which 
> touches exactly this area. It revolves more around the SG_IO handling;

Technically this patch is for *non* passthrough, but yeah it's a similar 
case.

> qemu ignores the host_status setting completely, leading to data
> corruption if the host has to abort commands. (Not that the host
> _does_ abort commands, as qemu SG_IO sets an infinite timeout. But
> that's another story).

> And part of the patchset is to enable passing of the host_status code 
> back to the drivers. In particular virtio_scsi has a 'response' code 
> which matches pretty closely to the linux SCSI DID_XXX codes.

Yeah, most of the time that's just because it's what can go wrong in 
SCSI.  Sometimes it's because I had no clue when writing the virtio-scsi 
spec and just copied blindly from Linux.  For example 
VIRTIO_SCSI_S_NEXUS_FAILURE probably should have never existed, since 
DID_NEXUS_FAILURE really should have been DID_RESRVATION_CONFLICT.

As an aside, I hate Linux host_status.  It's never clear when looking at 
the code if the statuses have been mapped back to BLK_STS codes or not, 
so you don't know if you already have gotten rid of DID_TARGET_FAILURE, 
DID_NEXUS_FAILURE, DID_ALLOC_FAILURE and DID_MEDIUM_ERROR (which are 
just weird way to store the SCSI status or sense key for future use, and 
not really "host statuses), and would really be a nexus failure only in 
the rare case of path-specific reservations).

> So my idea is to pass the host_status directly down to the drivers, 
> allowing virtio-scsi to do a mapping between DID_XX and virtio response 
> codes.

But yeah, this is a good idea.  But since I hate host_status, please 
define your own enum instead of DID_*.  Of course you can use the same 
values as DID_* and assert with QEMU_BUILD_BUG_ON that they are the 
same, but I don't want people to read the code and have to think of 
DID_ALLOC_FAILURE and the like.

Paolo