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

[Keith Busch]

On Wed, Jun 24, 2020 at 11:33:25AM -0700, Sagi Grimberg wrote:
> > > 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.