nvme_sim: Attach as a child of nvme

[imp]

Summary

Rather than registering as a consumer of the nvme controller, hook into
the child device and use that.

This is a small regression at the moment: we don't fail the device when
that happens at runtime, and we don't handle new namespaces when they
arrive (though that feature is currently fragile).

Sponsored by: Netflix

[jhb]

sys/dev/nvme/nvme_sim.c

373–379

Have you thought about defining a nvme_if.m and having a nvme_ns_changed DEVMETHOD? You could then handle enumerating namespaces in the controller driver and just invoke the method in the child device each time a namespace changes.

(FWIW, nvmf(4) does handle namespaces coming and going dynamically using an AER to notice when a namespace changes. It works in that you can use ctladm to add new LUNs on the fly on the target side, and the FreeBSD nvmf client will notice the new namespace.)

[jhb]

I also don't understand the regression described in the commit message. (What do you mean by "that" in "failing devices when that happens at runtime"? Is "that" an AER, is it an I/O error, completion timeout?)

BTW, should this mean that you can kldunload nvme_sim and then kldload nvd to switch from nda to nvd? Or adjust the tunable in kenv and then do a 'devctl detach/attach' of the device? (Granted, I don't think devctl can name this device for attach to work)

[imp]

In D51384#1175594, @jhb wrote:

I also don't understand the regression described in the commit message. (What do you mean by "that" in "failing devices when that happens at runtime"? Is "that" an AER, is it an I/O error, completion timeout?)

BTW, should this mean that you can kldunload nvme_sim and then kldload nvd to switch from nda to nvd? Or adjust the tunable in kenv and then do a 'devctl detach/attach' of the device? (Granted, I don't think devctl can name this device for attach to work)

This commit regresses calling the failure callback that the 'consumer' interface has when the controller fails (and calls the right failure routine). One could, in theory, attach different types of nda / nvd devices after this change, but I've not tested to see if it breaks things... nvme_sim isn't a loadable module, it's co-located in the nvme module when cam is enabloed. These things should be possible, but are not likely to be seamless just yet..

sys/dev/nvme/nvme_sim.c

373–379

There's a future review that does exactly this sort of thing.

nvd doesn't handle dynamic namespaces at all... That future change will allow them to arrive.
nda allows them to come, but not depart or change

[jhb]

sys/dev/nvme/nvme_sim.c

373–379

Hmm, nda handles it fine for me with nvmf when I've tested it. At least removing devices does work.

[imp]

sys/dev/nvme/nvme_sim.c

373–379

Hmmm, reading the code suggests it shouldn't...
But maybe I fixed this case better than I'd thought when I was working on dynamic namespace support.... Adding works I know, and removing and adding back kinda works... I didn't think I had all the kinks out of simply removing though.

[imp]

sys/dev/nvme/nvme_sim.c

373–379

What also doesn't work well today is when the namespace size changes, we get this event, but we don't re-read the new size. This happens in AWS.

[imp]

Update per review, add missing change

[imp]

Update per review and rebase over resize changes... woof.