On Wed, Jun 24, 2020 at 11:33:25AM -0700, Sagi Grimberg wrote:

quoted

quoted

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.