libnvme: drop support for hostid config by igaw · Pull Request #3221 · linux-nvme/nvme-cli

igaw · 2026-03-31T12:40:31Z

The NVMe subsystem of Linux expects a 1:1 mapping of the hostnqn to hostid. There is no point in exposing the hostid argument through all public APIs.

To ensure backward compatibility, always set the hostid extracted from the hostnqn when building the connect string. This matches the default behavior prior to removing hostid from the API.

Copilot

Pull request overview

Removes hostid as a configurable/publicly-exposed concept in libnvme and related tooling, aligning with the Linux NVMe expectation of a 1:1 hostnqn↔hostid mapping by deriving hostid from the hostnqn where needed.

Changes:

Drop hostid from public C APIs, accessors, JSON config/schema, and Python bindings; update call sites and tests accordingly.
Stop emitting/expecting hostid in JSON output and golden test data.
Update nvme-cli documentation/CLI help to mark --hostid as deprecated/ignored.

Reviewed changes

Copilot reviewed 30 out of 30 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
nvme-print-json.c	Removes HostID from JSON printing of host/subsystem structures.
libnvme/test/tree.c	Updates tests for new `nvme_get_host()` signature (no hostid).
libnvme/test/sysfs/data/tree-pcie.out	Updates expected JSON output to no longer include hostid.
libnvme/test/sysfs/data/tree-apple-nvme.out	Updates expected JSON output to no longer include hostid.
libnvme/test/config/hostnqn-order.c	Updates config-resolution tests to resolve only hostnqn.
libnvme/test/config/data/tls_key-2.out	Updates expected JSON output to no longer include hostid.
libnvme/test/config/data/tls_key-1.out	Updates expected JSON output to no longer include hostid.
libnvme/test/config/data/hostnqn-order.json	Removes hostid from sample JSON config input.
libnvme/test/config/data/config-pcie-with-tcp-config.out	Updates expected JSON output to no longer include hostid.
libnvme/src/nvme/tree.h	Updates public header docs/signatures: drop hostid, add `nvme_host_resolve_hostnqn()`.
libnvme/src/nvme/tree.c	Implements `nvme_host_resolve_hostnqn()` and removes hostid handling from host lookup/lifecycle.
libnvme/src/nvme/private.h	Removes hostid from `struct nvme_host`; adjusts internal fabrics options/context fields; adds internal helpers.
libnvme/src/nvme/linux.h	Removes public declarations for hostid generation/reading helpers.
libnvme/src/nvme/linux.c	Refactors hostid/hostnqn helper logic and removes hostid file-reading API.
libnvme/src/nvme/json.c	Removes parsing/emitting `hostid` from JSON config/tree operations.
libnvme/src/nvme/fabrics.h	Updates fabrics context API to set only hostnqn (no hostid).
libnvme/src/nvme/fabrics.c	Derives hostid from hostnqn for connect strings and adjusts discovery/config/NBFT logic to no longer store hostid.
libnvme/src/nvme/accessors.h	Removes public accessors for host.hostid and fabric_options.hostid.
libnvme/src/nvme/accessors.c	Removes implementations of removed hostid-related accessors.
libnvme/src/libnvme.ld	Updates exported symbol list to remove hostid APIs and add `nvme_host_resolve_hostnqn`.
libnvme/src/accessors.ld	Updates exported accessor symbols to remove hostid accessors.
libnvme/libnvme/tests/test-objects.py	Updates Python tests to remove hostid-related expectations.
libnvme/libnvme/nvme.i	Updates SWIG interface: remove hostid property/ctor arg; adjust stringification.
libnvme/examples/discover-loop.c	Updates example code for new `nvme_get_host()` signature.
libnvme/doc/config-schema.json.in	Removes `hostid` from documented JSON schema.
fabrics.c	Updates nvme-cli fabrics frontend to call updated libnvme API and mark hostid as deprecated/ignored.
Documentation/nvme-discover.txt	Marks `--hostid` as deprecated/ignored.
Documentation/nvme-connect.txt	Marks `--hostid` as deprecated/ignored.
Documentation/nvme-connect-all.txt	Marks `--hostid` as deprecated/ignored.
Documentation/nvme-config.txt	Marks `--hostid` as deprecated/ignored.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-31T13:52:30Z

 	exat->exattype = cpu_to_le16(NVMF_EXATTYPE_HOSTID);
 	exat->exatlen  = cpu_to_le16(nvmf_exat_len(NVME_UUID_LEN));
-	nvme_uuid_from_string(h->hostid, exat->exatval);
+	hostid = nvme_hostid_from_hostnqn(h->hostnqn);
+	nvme_uuid_from_string(hostid, exat->exatval);


nvmf_fill_die() derives hostid via nvme_hostid_from_hostnqn() and then unconditionally passes it to nvme_uuid_from_string(). If h->hostnqn does not contain a uuid: component, nvme_hostid_from_hostnqn() returns NULL and this will dereference NULL inside sscanf(), causing a crash. Please add validation (and error propagation/logging) when hostid is NULL or when nvme_uuid_from_string() returns -EINVAL; if HostID is mandatory here, consider making nvmf_fill_die() return an int so callers can fail gracefully.

Copilot · 2026-03-31T13:52:30Z

+		else if (*entry->nbft->host.id)
+			hostnqn = nvme_hostnqn_from_hostid(
+				(const char*)entry->nbft->host.id);
+


In the NBFT host selection, entry->nbft->host.id appears to be a binary UUID buffer, but it is cast to const char * and passed to nvme_hostnqn_from_hostid(), which expects a NUL-terminated UUID string for %s. This can read past the buffer / produce invalid host NQNs. Convert the binary UUID to a string first (e.g., via nvme_uuid_to_string()), or add a helper that accepts a 16-byte UUID and formats it safely.

Suggested change

else if (*entry->nbft->host.id)

hostnqn = nvme_hostnqn_from_hostid(

(const char*)entry->nbft->host.id);

else if (*entry->nbft->host.id) {

char hostid_str[37];

nvme_uuid_to_string(entry->nbft->host.id, hostid_str);

hostnqn = nvme_hostnqn_from_hostid(hostid_str);

}

Copilot · 2026-03-31T13:52:30Z


 	hostnqn = nvme_host_get_hostnqn(h);
-	hostid = nvme_host_get_hostid(h);
+	hostid = nvme_hostid_from_hostnqn(hostnqn);


build_options() now derives the connect hostid from hostnqn with nvme_hostid_from_hostnqn(hostnqn), but if the host NQN is not of the ...:uuid:<uuid> form this returns NULL and the hostid option is silently omitted. If the goal is to preserve legacy behavior and ensure the kernel sees a HostID, handle the NULL case explicitly (e.g., generate a HostID and/or fail with a clear error so connects don't change behavior unexpectedly).

Suggested change

hostid = nvme_hostid_from_hostnqn(hostnqn);

hostid = nvme_hostid_from_hostnqn(hostnqn);

if (!hostid) {

/* Fallback to a configured hostid if the NQN does not embed one */

hostid = nvme_host_get_hostid(h);

if (!hostid)

nvme_msg(h->ctx, LOG_WARNING,

"no HostID derived from hostnqn and no configured HostID; HostID will not be set\n");

}

Copilot · 2026-03-31T13:52:31Z

 * The order is:
 *
 *  - Start with informartion from DMI or device-tree
- *  - Override hostnqn and hostid from /etc/nvme files
- *  - Override hostnqn or hostid with values from JSON
- *    configuration file. The first host entry in the file is
- *    considered the default host.
- *  - Override hostnqn or hostid with values from the command line
- *    (@hostnqn_arg, @hostid_arg).
+ *  - Override hostnqn from the /etc/nvme/hostnqn file
+ *  - Override hostnqn value from JSON configuration file.
+ *    The first host entry in the file is considered the default host.
+ *  - Override hostnqn value from the command line @hostnqn_arg.


The documented resolution order for nvme_host_resolve_hostnqn does not match the implementation in tree.c: the code checks command-line arg, then JSON first-host, then /etc/nvme/hostnqn, then generates a hostnqn (which may use DMI/device-tree). Please update the comment to reflect the actual precedence so callers understand which source wins. Also, "informartion" is misspelled.

The NVMe subsystem of Linux expects a 1:1 mapping of the hostnqn to hostid. There is no point in exposing the hostid argument through all public APIs. To ensure backward compatibility, always set the hostid extracted from the hostnqn when building the connect string. This matches the default behavior prior to removing hostid from the API. Signed-off-by: Daniel Wagner <wagi@kernel.org>

The --hostid option is deprecated and ignored. Update the documentation accordingly. Signed-off-by: Daniel Wagner <wagi@kernel.org>

igaw · 2026-03-31T14:28:27Z

Obviously, there are two supported NQN formats and this here would support for the support for the first format... argh.

igaw · 2026-03-31T16:15:02Z

This is wrong, working on a different fix, so it's not necessary to provide --hostid when --hostnqn is in the second format.

igaw force-pushed the drop-host-id branch 6 times, most recently from 337926e to c6335d3 Compare March 31, 2026 13:40

igaw requested a review from Copilot March 31, 2026 13:43

Copilot started reviewing on behalf of igaw March 31, 2026 13:43 View session

Copilot AI reviewed Mar 31, 2026

View reviewed changes

igaw added 2 commits March 31, 2026 16:05

docs: add deprecation not to --hostid

478ce28

The --hostid option is deprecated and ignored. Update the documentation accordingly. Signed-off-by: Daniel Wagner <wagi@kernel.org>

igaw force-pushed the drop-host-id branch from c6335d3 to 478ce28 Compare March 31, 2026 14:06

igaw closed this Mar 31, 2026

igaw deleted the drop-host-id branch April 2, 2026 10:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

libnvme: drop support for hostid config#3221

libnvme: drop support for hostid config#3221
igaw wants to merge 2 commits intolinux-nvme:masterfrom
igaw:drop-host-id

igaw commented Mar 31, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Uh oh!

igaw commented Mar 31, 2026

Uh oh!

igaw commented Mar 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-		else if (*entry->nbft->host.id)
-			hostnqn = nvme_hostnqn_from_hostid(
-				(const char*)entry->nbft->host.id);
+		else if (*entry->nbft->host.id) {
+			char hostid_str[37];
+			nvme_uuid_to_string(entry->nbft->host.id, hostid_str);
+			hostnqn = nvme_hostnqn_from_hostid(hostid_str);
+		}

-	hostid = nvme_hostid_from_hostnqn(hostnqn);
+	hostid = nvme_hostid_from_hostnqn(hostnqn);
+	if (!hostid) {
+		/* Fallback to a configured hostid if the NQN does not embed one */
+		hostid = nvme_host_get_hostid(h);
+		if (!hostid)
+			nvme_msg(h->ctx, LOG_WARNING,
+				 "no HostID derived from hostnqn and no configured HostID; HostID will not be set\n");
+	}

Conversation

igaw commented Mar 31, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

igaw commented Mar 31, 2026

Uh oh!

igaw commented Mar 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants