Re: [PATCH] lpfc: use NVMe error codes for LS request done callback

[James Smart <james.smart@broadcom.com>]

[-- Attachment #1: Type: text/plain, Size: 1803 bytes --]

On 9/16/2020 10:37 PM, Hannes Reinecke wrote:
> On 9/16/20 11:23 PM, James Smart wrote:
>>
>>
>> On 9/16/2020 1:50 AM, Hannes Reinecke wrote:
>>> The LS request callback requires a 'status' argument, but that one
>>> should be an NVMe error code, not a driver specific one which has
>>> no meaning in the nvme layer.
>>>
>>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>>> ---
>>>   drivers/scsi/lpfc/lpfc_nvme.c | 4 +++-
>>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/scsi/lpfc/lpfc_nvme.c 
>>> b/drivers/scsi/lpfc/lpfc_nvme.c
>>> index e5be334d6a11..4b007a28014b 100644
>>> --- a/drivers/scsi/lpfc/lpfc_nvme.c
>>> +++ b/drivers/scsi/lpfc/lpfc_nvme.c
>>> @@ -498,7 +498,9 @@ __lpfc_nvme_ls_req_cmp(struct lpfc_hba *phba,  
>>> struct lpfc_vport *vport,
>>>           cmdwqe->context3 = NULL;
>>>       }
>>>       if (pnvme_lsreq->done)
>>> -        pnvme_lsreq->done(pnvme_lsreq, status);
>>> +        pnvme_lsreq->done(pnvme_lsreq,
>>> +                  status == IOSTAT_SUCCESS ?
>>> +                  NVME_SC_SUCCESS : NVME_SC_INTERNAL);
>>>       else
>>>           lpfc_printf_vlog(vport, KERN_ERR, LOG_TRACE_EVENT,
>>>                    "6046 NVMEx cmpl without done call back? "
>>
>> No - it's not a nvme command, so doesn't need a nvme status code. It 
>> should be a -Exxx  value or a 0 (success). nvme_fc_send_ls_req() for 
>> example calls __nvme_fc_send_ls_req(), which can return any number of 
>> -Exxx values, and the routine returns the value returned by the done 
>> call after waiting for it to complete - so they should all follow the 
>> same form.
>>
> Right. But returning IOSTAT definitions is still wrong :->

true..  :)

-- james


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4163 bytes --]