libnvme/nvme-cli: add `nvme config-convert`, the legacy config to INI migration tool by martin-belanger · Pull Request #3590 · linux-nvme/nvme-cli

martin-belanger · 2026-07-16T16:42:39Z

This is the first thing that actually wires up to the INI read/write API added in #3566 and #3587: a one-shot, explicit command that converts an existing config.json and/or discovery.conf into the new INI format.

It's also just the first step. The plan is for a follow-on PR to add implicit conversion: connect-all/discover converting legacy config to INI automatically on first invocation, so INI becomes the format libnvme actually reads, with the conversion happening transparently at the CLI layer before any libnvme call. That implicit path is separate work, not part of this PR -- this one only adds the explicit command: nvme config-convert.

Commits:

nvme-cli: add the config.json reader for INI conversion
fabrics, nvme-cli: add the discovery.conf reader for INI conversion
nvme-cli: add the config-convert command
libnvme/design: fix stale config-convert references in CONFIG.md
doc: add nvme-config-convert man page

nvme config-convert converts config.json and/or discovery.conf to the INI format read by nvme connect-all/nvme discover. With no --config, it converts whichever of the two well-known legacy files exist, matching what a bare connect-all already reads today; --config restricts it to one JSON file. Refuses to overwrite an existing target unless --force is given. On success, the converted legacy file(s) are renamed to <name>.converted so a repeated run doesn't convert them again.

Almost all of this lives in config-convert.c/config-convert.h, deliberately. Since support for the legacy configuration files is temporary, the conversion logic is kept isolated so it can eventually be removed with minimal impact. Unfortunately, not all the code could be put in config-convert.c. The only exception is the discovery.conf argv-style parser (nvmf_convert_discovery_line), which stays in fabrics.c to reuse the same option table as nvme discover; everything else lives in config-convert.c.

Ran the comments and man page through ChatGPT for a "man page style, simple/precise language" pass, then reviewed the result by hand.

Depends on #3587 (merged).

Parses the legacy config.json format directly into PR2's builder (libnvmf_config_emit_add()), for the upcoming "nvme config convert" migrator. Lives in nvme-cli, json-c-gated there, since libnvme drops json.c and conversions must keep working for years of 2.x->3.x upgrades. Not wired into any command yet. Underscored legacy keys map to the hyphenated INI spellings via a small lookup table. dhchap_key is spec-scoped to (hostnqn, subsysnqn), never per path (Base Spec 2.3 8.3.5.5.7), so a port without its own value inherits config.json's host-level default; a port with its own differing value keeps it and logs a note, since that combination was already non-compliant in the source file. Addresses are never resolved or rejected -- a hostname traddr is written to the INI verbatim, same as any human-authored entry. Signed-off-by: Martin Belanger <martin.belanger@dell.com>

discovery.conf lines are argv-style syntax ("-t rdma -a 192.168.1.4 -s 4420 -q <hostnqn>" or "--transport=tcp --traddr=..."), the same NVMF_ARGS table 'nvme discover' already parses. Add nvmf_convert_discovery_line() in fabrics.c, next to the existing hook_parser_next_line() it's modeled on, so both share the exact same option table instead of a second, drifting parser. It builds a libnvmf_params set from the recognized tunables and adds one discovery controller entry to the emitter per line; a blank, comment, or incomplete line is silently skipped, matching hook_parser_next_line()'s own tolerance. nvme_config_convert_discovery() in config-convert.c drives the file read and calls it per line -- needs no json-c, unlike the config.json reader. Signed-off-by: Martin Belanger <martin.belanger@dell.com>

igaw · 2026-07-17T08:06:01Z

Looks good. Nice to see how simple the convert function is. Great job!

igaw · 2026-07-17T08:54:36Z

Could we also have some end to end tests for this feature (those in the tests directory?) We need to get this feature as correct as possible, including the corner cases if possible.

martin-belanger · 2026-07-17T11:08:23Z

Thanks for the review.

Moved the "nothing to convert" check out of the else block, per your comment on config-convert.c:531. It fell out naturally from the fix below rather than needing a standalone change.
Fixed the "one shot" issue (config-convert.c:538): discovery.conf conversion is no longer gated by --config. It matches nvme connect-all's existing behavior now -- the well-known discovery.conf path is always converted when it exists, regardless of whether --config points at a non-default config.json. --config only ever affects which JSON file gets converted.
Added end-to-end tests in tests/nvme_config_convert_test.py: DC/subsystem conversion, the legacy bare-array JSON format, dhchap-secret default inheritance and per-port override, and the error/force/rename paths.

One limitation on the test coverage worth flagging: discovery.conf's own conversion isn't exercised end-to-end. Its path has no --config-style override, so a test can only hit it by touching the real /etc/nvme/discovery.conf -- and that file exists on essentially every system, even freshly installed ones, since we ship an empty (comment-only) stub by default. Converting it end-to-end in a test would mean renaming that file on whatever machine runs the suite, which isn't something a test should do. The suite instead skips itself whenever that file is present, and I verified the logic separately against the real binary with the guard bypassed.

Related note, not for this PR: we should probably stop installing that empty discovery.conf stub by default (meson.build and nvme.spec.in both ship it unconditionally today). It's what made the test-coverage gap above universal rather than a corner case. Tracking as a separate follow-up.

igaw · 2026-07-17T12:08:34Z

Yes, we should remove the discovery.conf install step now. This should also be documented eventually. My current plan is to start documenting this for a release note file. All the stuff which could regress should be mentioned there.

martin-belanger · 2026-07-17T12:14:58Z

Sounds good — happy to help once you've got a structure in mind. What format are you picturing (a single RELEASE_NOTES.md, one entry per PR, something else)? I can contribute entries for the changes on my end (this discovery.conf removal, and whatever else from the INI/config pipeline is worth flagging) once there's a shape to fit them into.

igaw · 2026-07-17T14:08:57Z

Sounds good — happy to help once you've got a structure in mind. What format are you picturing (a single RELEASE_NOTES.md, one entry per PR, something else)? I can contribute entries for the changes on my end (this discovery.conf removal, and whatever else from the INI/config pipeline is worth flagging) once there's a shape to fit them into.

I don't have an exact idea how it should be handled, I think when a new feature or so lands. I'd say we just copy the strategy/format from an existing project, systemd?

What about RELEASE.md

Anyway w oould make my life simpler :)

Wires the config.json and discovery.conf readers into a migration tool. --config converts a config.json at a non-default path in place of the default; discovery.conf conversion is never gated by --config -- the well-known discovery.conf path is always converted when it exists, matching what a bare "nvme connect-all" does today. Installs to nvme-fabrics.conf by default, or --output; refuses to overwrite an existing configuration and renames each converted legacy file to <name>.converted. A rerun against a source already converted on a prior run (renamed to <name>.converted) is a no-op, not an error -- only a source that was never converted at all is treated as missing. Signed-off-by: Martin Belanger <martin.belanger@dell.com>

The command settled on the hyphenated "nvme config-convert" rather than the two-word form this doc still used, and converted legacy files are renamed to <name>.converted rather than left in place. Signed-off-by: Martin Belanger <martin.belanger@dell.com>

Signed-off-by: Martin Belanger <martin.belanger@dell.com>

run_cmd() and the base logging setup were only reachable through TestNVMe, which requires a real controller (config.json's controller/ns1, PCI validation, namespace management). A device-free CLI test has no way to reuse them without dragging that in too. Split TestNVMe into TestNVMeBase (run_cmd, logging setup, no device needed) and TestNVMe (adds the device-specific setup on top). run_cmd() also gains a shell= kwarg, defaulting to True to match every existing call site exactly; passing shell=False runs an argv list directly with no shell involved, for callers that don't need shell features like the pipes/redirects a couple of existing tests rely on. Signed-off-by: Martin Belanger <martin.belanger@dell.com>

Exercise the CLI via --config/--output under a temp directory: DC and subsystem conversion, the legacy bare-array JSON format, dhchap-secret default inheritance and per-port override, the error/force/rename paths, and that a rerun after a prior successful conversion is idempotent rather than erroring on the now-missing source file. discovery.conf's own conversion is not covered here: its path has no override, so the suite skips itself whenever a real /etc/nvme/discovery.conf exists rather than risk converting it. Built on TestNVMeBase for the same run_cmd/logging infra the hardware-backed tests use. Signed-off-by: Martin Belanger <martin.belanger@dell.com>

martin-belanger · 2026-07-17T14:38:36Z

All done.
I will have a closer look at what systemd does when they have a new relase. From what I can tell they use a "NEWS" file.

Martin Belanger added 2 commits July 16, 2026 12:57

martin-belanger force-pushed the libnvme-config-json-convert branch from b3ab97e to 70d26de Compare July 16, 2026 17:03