nvmecontrol: New commands to support Fabrics hosts
Needs ReviewPublic
Actions

Authored by jhb on Tue, Apr 9, 11:03 PM.

Details

Reviewers

imp

Summary

discover: Connects to a remote Discovery controller, fetches its Discovery Log Page, and enumerates the remote controllers described in the log page.

The -v option can be used to display the Identify Controller data structure for the Discovery controller. This is only really useful for debugging.

connect: Connects to a remote I/O controller establish an association of an admin queue and a single I/O queue. The association is handed off to the in-kernel host to create a new nvmeX device.

The -d option can be used to connect to a Discovery controller and attempt to create an association with each I/O controller enumerated in the Discovery controller's Discovery Log Page.

reconnect: Establishes a new association with a remote I/O controller for an existing nvmeX device. This can be used to restore access to a remote I/O controller after the loss of a prior association due to a transport error, controller reboot, etc.

disconnect: Deletes an nvmeX device after detaching its namespaces and terminating any active association.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 57191
Build 54079: arc lint + arc unit

Event Timeline

jhb created this revision.Tue, Apr 9, 11:03 PM

Herald added a subscriber: dab. · View Herald TranscriptTue, Apr 9, 11:03 PM

jhb requested review of this revision.Tue, Apr 9, 11:03 PM

Harbormaster completed remote builds in B57006: Diff 136811.Tue, Apr 9, 11:03 PM

jhb added a parent revision: D44714: nvmf: The in-kernel NVMe over Fabrics host.Tue, Apr 9, 11:03 PM

jhb added a child revision: D44716: ctl: Avoid an upcast for calling ctl_scsi_path_string.

Generally I like it.
I'd have been tempted to have a fabric command and these as subcommands. Did you consider this and if so what was your thinking for taking this path. And if not, what do you think of the idea.

sbin/nvmecontrol/connect.c
76	what's missing here?
96	what's your thinking for continuing after this, here and elsewhere?

In D44715#1021069, @imp wrote:

Generally I like it.
I'd have been tempted to have a fabric command and these as subcommands. Did you consider this and if so what was your thinking for taking this path. And if not, what do you think of the idea.

The current usage is somewhat akin to nvme(1) on Linux which has top-level connect, discover, etc. commands. The options passed after the command name are different, but the commands are named the same. It's also true though that Linux's nvme(1) doesn't have any subcommands (e.g. it has detach-ns instead of ns detach). Linux doesn't use connect-fabric though, it is just connect and connect-all (the latter similar to connect -d for nvmecontrol(8)).

In D44715#1021543, @jhb wrote:

In D44715#1021069, @imp wrote:

Generally I like it.
I'd have been tempted to have a fabric command and these as subcommands. Did you consider this and if so what was your thinking for taking this path. And if not, what do you think of the idea.

The current usage is somewhat akin to nvme(1) on Linux which has top-level connect, discover, etc. commands. The options passed after the command name are different, but the commands are named the same. It's also true though that Linux's nvme(1) doesn't have any subcommands (e.g. it has detach-ns instead of ns detach). Linux doesn't use connect-fabric though, it is just connect and connect-all (the latter similar to connect -d for nvmecontrol(8)).

OK. That makes some sense...

Is there a reason there's a different command line structure? The newer nvmecontrol commands I've implemented I've tried to be as close to Linux as is reasonable to make it easier for us to cope with vendor recipes... Though not always, so it's kinda a hodge podge...

This revision is now accepted and ready to land.Tue, Apr 16, 7:25 PM

Move nvmf_transport_type to libnvmf

This revision now requires review to proceed.Tue, Apr 16, 8:45 PM

Harbormaster completed remote builds in B57135: Diff 137180.Tue, Apr 16, 8:45 PM

Rebase

Harbormaster completed remote builds in B57191: Diff 137330.Thu, Apr 18, 5:54 PM

In D44715#1021587, @imp wrote:

In D44715#1021543, @jhb wrote:

In D44715#1021069, @imp wrote:

Generally I like it.
I'd have been tempted to have a fabric command and these as subcommands. Did you consider this and if so what was your thinking for taking this path. And if not, what do you think of the idea.

The current usage is somewhat akin to nvme(1) on Linux which has top-level connect, discover, etc. commands. The options passed after the command name are different, but the commands are named the same. It's also true though that Linux's nvme(1) doesn't have any subcommands (e.g. it has detach-ns instead of ns detach). Linux doesn't use connect-fabric though, it is just connect and connect-all (the latter similar to connect -d for nvmecontrol(8)).

OK. That makes some sense...

Is there a reason there's a different command line structure? The newer nvmecontrol commands I've implemented I've tried to be as close to Linux as is reasonable to make it easier for us to cope with vendor recipes... Though not always, so it's kinda a hodge podge...

Hmm, I'll see how close I can get it perhaps. Linux calls connect-d connect-all and also has a disconnect-all. Some differences will be inherent I think since Linux doesn't have new-bus so their disconnect can't be the same, but I will see if I can more closely align them.