libnvme/config: add an INI-format NVMe-oF config parser

[martin-belanger]

Hi Daniel,

Here's the big PR I promised — ~4,400 lines total, but only ~1,400 of that is actual code and another ~1,400 tests; the rest is kdoc/comments, blank lines, and the design doc. It's self-contained: nothing calls into it yet, so no regression risk today. It's this size because it does two things the JSON blob didn't: real validation with file:line diagnostics, and a drop-in file layout.

Suggested order:

1. CONFIG.md first (the design contract),
2. then config.h (the whole public surface),
3. then the commits in order:
   3.1 tokenizer: libnvme/config: add an internal INI reader
   3.2 param bag: libnvme/config: add the parameter bag and key table
   3.3 raw parser: libnvme/config: parse a config file into its raw model
   3.4 cascade resolver: libnvme/config: resolve the cascade into the flat connection list
   3.5 public API: libnvme/config: publish the resolved configuration
   3.6 arg emitter: libnvme/config: add the connect argument emitter
4. Last commit is unrelated — a pre-existing host_iface/trsvcid copy-paste mixup in libnvmf_add_ctrl(), fixed while I was in the area.

A second (much smaller) PR (write side + nvme config convert) follows once this one lands.

[igaw]

Generally, I really like this. It's a big PR but that was to be expected. And it's good to see the whole picture. Also the diagnostic part is a nice feature to have. The public API is not that big after realizing what is exposed and what not. Good job!

The main points from my first review pass is mostly nitpicking (function signatures and not using ccan list)

My big question is about config.json: is this correctly modeled? From my current understanding it is not... and the other question is: do we want to keep /etc/nvme/hostnqn and /etc/nvme/hostid alive?

[martin-belanger]

and the other question is: do we want to keep /etc/nvme/hostnqn and /etc/nvme/hostid alive?

hostnqn/hostid — before answering "keep or drop the /etc/nvme files," I think there's a more basic question: should libnvme be picking a hostnqn/hostid by itself at all? Traced it — lookup_host() (core connect path, fabrics.c) calls libnvmf_host_get_ids(), which has a whole baked-in precedence chain: DMI/device-tree → /etc/nvme files → config.json's first host → CLI args → random-generate if still nothing. That's libnvme deciding policy, same category of thing we already pulled out for hostname resolution. Feels like it should go the same way: libnvme requires an explicit hostnqn+hostid to connect, full stop, and every orchestrator (CLI included) is on the hook to supply one — however it gets it, /etc/nvme, a config file, DMI, or freshly generated. What's your read?

[igaw]

and the other question is: do we want to keep /etc/nvme/hostnqn and /etc/nvme/hostid alive?

hostnqn/hostid — before answering "keep or drop the /etc/nvme files," I think there's a more basic question: should libnvme be picking a hostnqn/hostid by itself at all? Traced it — lookup_host() (core connect path, fabrics.c) calls libnvmf_host_get_ids(), which has a whole baked-in precedence chain: DMI/device-tree → /etc/nvme files → config.json's first host → CLI args → random-generate if still nothing. That's libnvme deciding policy, same category of thing we already pulled out for hostname resolution. Feels like it should go the same way: libnvme requires an explicit hostnqn+hostid to connect, full stop, and every orchestrator (CLI included) is on the hook to supply one — however it gets it, /etc/nvme, a config file, DMI, or freshly generated. What's your read?

Generally, I'd say yes. Though there is a lot of complexity currently backed into libnvmf_host_get_ids and the code around it. The idea behind this function is that not every client of the API has to implement the same kind of lookup code, e.g. for the Python binding users. I agree it might be better to move this out and expect the correct hostnqn/hostid is provided on the 'connect/discover' command. Would it make sense to provide this function as util function?

[martin-belanger]

Would it make sense to provide this function as util function?

Like the util-function idea — makes sense to promote libnvmf_host_get_ids() to the public API (it's currently internal-only, private-fabrics.h, not even SWIG-visible) rather than duplicate that lookup logic per consumer.

Plan, as a separate PR (not part of this one): lookup_host() stops calling it automatically — connect/discover require an explicit hostnqn/hostid, full stop, -EINVAL otherwise. The function itself moves to fabrics.h as a normal public util any caller can opt into, and the CLI becomes its first consumer: when --hostnqn/--hostid aren't given on the command line, nvme-cli will explicitly call this helper itself before invoking connect, instead of relying on it happening silently inside libnvme.

nvme-stas actually built its own version of this exact lookup years ago; would have reused this if it had existed then.

[igaw]

the commit message for the second patch is also difficult understand. It mentions 'bag' without really defining what it is and the the code it doesn't really show up, only in a comment. I've gave chatgpt the commit message to transform into more technical tone:

Ah, the term "bag" is from compiler terminology. It's been a while. I suppose it can stay then, I just forgot about compiler building, something other people do :)

[igaw]

I let chatgpt translate the commit message for libnvme/config: add an INI-format NVMe-oF config parse

libnvme/config: add an INI-format NVMe-oF config parser

Store sections and entries from a configuration file without applying
precedence or inheritance rules. The cascade resolver consumes these
records later, allowing parsing and policy decisions to remain separate.

Global and per-type default sections are merged with a warning when
duplicated. Host sections are singletons, while endpoint sections may be
repeated. Controller entries keep the raw addressing strings and separate
per-path tunable overrides. Unknown keys and security options on path
entries are rejected.

Syntax and structural errors fail parsing with a file:line diagnostic.
Non-fatal policy issues produce warnings and allow parsing to continue.

Keep addressing values as raw strings instead of converting them to TIDs.
Configuration files are a user-facing interface and may contain hostnames
in traddr or host-traddr fields. TID creation has stricter requirements and
belongs to the consumer that interprets the parsed configuration.

[igaw]

libnvme/config: resolve the cascade into the connection list

Resolve the configuration hierarchy into the flat connection list consumed
by users of the API. Load the main configuration file and its sorted
drop-ins, then apply the precedence rules to produce the final set of
(role, addressing, identity, params) connections.

Keep default parameter scopes attached to resolved discovery controllers.
This ensures controllers discovered through a DC (such as referrals or DLP
entries) inherit the correct DC and IOC defaults. mDNS-discovered DCs use the
top-level scope.

Enforce relational validation rules during resolution. Reject drop-in
personas without a name, duplicate host IDs assigned to different personas,
and host NQNs associated with multiple host IDs.

Keep addressing and identity fields as raw strings. traddr may still contain
a hostname at this stage because parsing does not perform resolution. TID
construction happens later in the consumer after address resolution.

[igaw]

libnvme/config: publish the resolved configuration API

Expose the resolved configuration through a public API. The API provides
configuration load, validation, and destruction, iteration over the resolved
connection list, per-connection accessors, and lookup of the default
parameters for controllers discovered at runtime (for example through DLP,
AEN, or mDNS).

Keep the parser and configuration hierarchy internal. Consumers see only the
resolved connection list, where each entry provides its role, addressing,
identity, and connection parameters.

Expose addressing and identity through per-field accessors rather than as a
TID. Address fields may still require name resolution, so TID construction is
left to the caller.

Publish the read-only connection parameter API because resolved connections
reference shared parameter sets instead of copying individual values.

Link the test against the installed shared library instead of the internal
test library to verify the exported symbols defined by the version script.

[igaw]

libnvme/config: add the connect argument emitter

Emit an NVMe connect argument list from a resolved (TID, parameters) pair.
The resolved configuration model provides the information needed to generate
the arguments for a single "nvme connect" invocation, which can be used by
consumers creating transient connections or displaying reproducible commands.

Emit addressing and identity fields first, followed by connection parameters.
Parameters with unset or reset values are omitted so the kernel defaults
remain in effect. Boolean parameters are emitted as flags.

Accept a TID and parameter set instead of a resolved connection object. This
allows the emitter to be used for both configured connections and
runtime-discovered controllers, where the caller constructs the TID and
obtains the parameters separately.

[martin-belanger]

@igaw -- Think I've addressed everything from this round, including the libnvmf_conf_file_parse() miss — I'd only converted libnvmf_config_load() last time and missed its sibling; it's now on the same int fn(..., struct foo **out) convention, both call sites and the test updated. Also reworded the commit messages for the INI parser, cascade resolver, publish, and emitter commits to your suggested text, and renamed the "parameter bag" to "parameter store" (fair point that "bag" isn't common terminology here).

Pushed. Let me know if I missed anything.