[PATCHv2 3/3] nvme: Expose cntrltype and dctype through sysfs

[Sagi Grimberg]

>> Hannes suggested that we combine both attributes into a single one that would take the values: "io", "discovery-none", "discovery-ddc", "discovery-cdc", "discovery-reserved", and "admin". But Sagi did not like this proposal and instead proposed the changes that you see here. Basically, we want to be able to re-execute the is_visible() method after we have identified the controller so that we can determine whether the dctype should be exposed.

Just to note that the same fundamental issue exists in both approaches,
having the "combined" string change after the controller is identified
is equivalent to having the separated attribute appear after the device
is initialized.

> Nope, that's not how the driver model works and you break all userspace
> tools by doing this as it only scans the list of attributes when the
> device is created and announced to userspace.
> 
> By changing the files and types and such afterward no one ever notices
> and so userspace is "blind" to the change unless you want it to
> constantly walk all of sysfs all the time to find this out (hint, you
> don't want that...)

The idea was to add a separate udev announcement that would tell
userspace that these attributes are stable (it actually serves
a different purpose, but it does come after that point).

> 
> So there's a reason you are being forced to add a driver-core change
> like this, because it's not something you should do :)

I do agree that its a bit backwards to do this update. But the device
even today is not fully usable when it is created, so we know no
userspace program relies on the creation announcement.

>> So now I'm in a bit of a pickle. Sagi suggested this current patch set, but from what you're saying this is not a good approach. If you could suggest a solution that would satisfy all parties, I would greatly appreciate it.
> 
> Wait to create the device when you know the device type.

The initial proposal was to defer the device creation to after the
device is fully initialized. That though is a heavier lift because the
device initialization is something that happens on every reset or
reconnection after connectivity loss, so if we move the device creation
there we have to now check if this is the first time we are doing this
or not, and also error recovery becomes more opinionated...