[PATCHv3 3/5] nvme: implement I/O Command Sets Command Set support

[Sagi Grimberg]

>>>> If the controller does not support the CNS value, it may return Invalid
>>>> Field with DNR set. That error currently gets propogated back to
>>>> nvme_init_ns_head(), which then abandons the namespace. Just as the code
>>>> coments say, we had been historically been clearing such errors because
>>>> we have other ways to identify the namespace, but now we're not clearing
>>>> that error.
>>>
>>> I don't understand what you are saying Keith.
>>>
>>> My comment was for this block:
>>> -- 
>>>       if (!status && nvme_multi_css(ctrl) && !csi_seen) {
>>>           dev_warn(ctrl->device, "Command set not reported for nsid:%d\n",
>>>                nsid);
>>>           status = -EINVAL;
>>>       }
>>> -- 
>>>
>>> I was saying that !status doesn't necessarily mean success, but it can
>>> also mean that its an retry-able error status (due to transport or
>>> controller). If we see a retry-able error we should still clear the
>>> status so we don't abandon the namespace.
>>>
>>> This for example would achieve that:
> 
> We're not talking about the same thing. I am only talking about what
> introduced the DNR check, from commit 59c7c3caaaf87.
> 
> I know you added it because you want to abort comparing identifiers on a
> rescan when retrieving the identifiers failed. That's fine, but I am
> saying this fails namespace creation in the first place for some types
> of devices that used to succeed.

OK, now I think I understand (thanks for clarifying that the discussion
is not on patch 3/5, but rather on 59c7c3caaaf87).

So the original proposal was to check NVME_SC_HOST_PATH_ERROR (and now
we have NVME_SC_HOST_ABORTED_CMD) but with the review Christoph gave
it seemed to make more sense that we generalize and check the DNR bit.

We could restrict it back to checking the status is
NVME_SC_HOST_PATH_ERROR or NVME_SC_HOST_ABORTED_CMD if you think it
creates problems. However, if we keep the code as is, the original
comment still holds.