[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Paolo Bonzini
Subject: Re: [PATCH] scsi-disk: convert more errno values back to SCSI statuses
Date: Fri, 13 Nov 2020 19:15:15 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.4.0

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.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]