Re: [PATCH 09/11] block, nvme: Add error for reservation conflicts.

[michael.christie@oracle.com]

On 6/4/22 2:38 AM, Hannes Reinecke wrote:
> On 6/3/22 21:45, Keith Busch wrote:
>> On Fri, Jun 03, 2022 at 01:55:34AM -0500, Mike Christie wrote:
>>> @@ -171,6 +171,7 @@ static const struct {
>>>       /* zone device specific errors */
>>>       [BLK_STS_ZONE_OPEN_RESOURCE]    = { -ETOOMANYREFS, "open zones exceeded" },
>>>       [BLK_STS_ZONE_ACTIVE_RESOURCE]    = { -EOVERFLOW, "active zones exceeded" },
>>> +    [BLK_STS_RSV_CONFLICT]    = { -EBADE,    "resevation conflict" },
>>
>> You misspelled "reservation". :)
>>
>> And since you want a different error, why reuse EBADE for the errno? That is
>> already used for BLK_STS_NEXUS that you're trying to differentiate from, right?
>> At least for nvme, this error code is returned when the host lacks sufficient
>> rights, so something like EACCESS might make sense.
>>
>> Looks good otherwise.
> 
> Welll ... BLK_STS_NEXUS _is_ the reservation error.

I was not sure of xen/virtio scsi uses of BLK_STS_NEXUS/DID_NEXUS_FAILURE.
The virtio spec's description for VIRTIO_SCSI_S_NEXUS_FAILURE:

    if the nexus is suffering a failure but retrying on other paths might
    yield a different result. 

looks like the description for DID_NEXUS_FAILURE in scsi_status.h.
To me the the description sounded generic where it could used for
other errors like the endpoint/port for the I_T is removed.

However, the qemu code only uses VIRTIO_SCSI_S_NEXUS_FAILURE for
reservation conflicts. If we are saying that is always the case in
other virt implementations, I don't even need this patch :) and we
can do what you requested and do more of a rename.