From 78f36fb27650c76c4792a49cdf28cacbdc4b96fa Mon Sep 17 00:00:00 2001 From: Martin Belanger Date: Wed, 15 Jul 2026 15:04:48 -0400 Subject: [PATCH 1/7] nvme-cli: add the config.json reader for INI conversion 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 --- config-convert.c | 297 +++++++++++++++++++++++++++++++++++++++++++++++ config-convert.h | 15 +++ meson.build | 1 + 3 files changed, 313 insertions(+) create mode 100644 config-convert.c create mode 100644 config-convert.h diff --git a/config-convert.c b/config-convert.c new file mode 100644 index 0000000000..5da6a9c2ba --- /dev/null +++ b/config-convert.c @@ -0,0 +1,297 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (c) 2026, Dell Technologies Inc. or its subsidiaries. + * + * Authors: Martin Belanger + */ + +#include +#include + +#include + +#include "common.h" +#include "config-convert.h" +#include "nvme-print.h" + +#ifdef CONFIG_JSONC +#include + +struct legacy_key { + const char *json_key; + const char *ini_key; +}; + +/* Map config.json keys from underscore to hyphen notation. */ +static const struct legacy_key int_keys[] = { + { "nr_io_queues", "nr-io-queues" }, + { "nr_write_queues", "nr-write-queues" }, + { "nr_poll_queues", "nr-poll-queues" }, + { "queue_size", "queue-size" }, + { "keep_alive_tmo", "keep-alive-tmo" }, + { "reconnect_delay", "reconnect-delay" }, + { "ctrl_loss_tmo", "ctrl-loss-tmo" }, + { "fast_io_fail_tmo", "fast-io-fail-tmo" }, + { "tos", "tos" }, +}; + +static const struct legacy_key bool_keys[] = { + { "duplicate_connect", "duplicate-connect" }, + { "disable_sqflow", "disable-sqflow" }, + { "hdr_digest", "hdr-digest" }, + { "data_digest", "data-digest" }, + { "tls", "tls" }, + { "concat", "concat" }, +}; + +static const struct legacy_key string_keys[] = { + { "tls_key", "tls-key" }, + { "tls_key_identity", "tls-key-identity" }, + { "keyring", "keyring" }, + { "dhchap_key", "dhchap-secret" }, + { "dhchap_ctrl_key", "dhchap-ctrl-secret" }, +}; + +static const char *map_key(const struct legacy_key *table, size_t n, + const char *json_key) +{ + size_t i; + + for (i = 0; i < n; i++) + if (!strcmp(table[i].json_key, json_key)) + return table[i].ini_key; + + return NULL; +} + +#define MAP_KEY(table, json_key) map_key(table, ARRAY_SIZE(table), json_key) + +static const char *json_get_string(struct json_object *obj, const char *key) +{ + struct json_object *val = json_object_object_get(obj, key); + + return val ? json_object_get_string(val) : NULL; +} + +static bool json_get_bool(struct json_object *obj, const char *key) +{ + struct json_object *val = json_object_object_get(obj, key); + + return val && json_object_get_boolean(val); +} + +/* Copy supported tunable and security parameters into @params. */ +static void apply_port_params(struct libnvmf_params *params, + struct json_object *port_obj) +{ + json_object_object_foreach(port_obj, key_str, val_obj) { + const char *ini_key; + char buf[32]; + + ini_key = MAP_KEY(int_keys, key_str); + if (ini_key) { + snprintf(buf, sizeof(buf), "%d", + json_object_get_int(val_obj)); + libnvmf_params_set(params, ini_key, buf); + continue; + } + ini_key = MAP_KEY(bool_keys, key_str); + if (ini_key) { + libnvmf_params_set(params, ini_key, + json_object_get_boolean(val_obj) ? + "true" : "false"); + continue; + } + ini_key = MAP_KEY(string_keys, key_str); + if (ini_key) + libnvmf_params_set(params, ini_key, + json_object_get_string(val_obj)); + } +} + +/* + * The DH-HMAC-CHAP secret is defined per (hostnqn, subsysnqn), not per path + * (NVMe Base Specification 2.3, section 8.3.5.5.7). If a port does not + * specify a secret, inherit the host-level default. If both are present but + * differ, keep the port-specific value and log the mismatch. + */ +static void apply_dhchap_default(struct libnvmf_params *params, + struct json_object *port_obj, const char *host_default) +{ + const char *port_value = json_get_string(port_obj, "dhchap_key"); + + if (!host_default) + return; + + if (!port_value) { + libnvmf_params_set(params, "dhchap-secret", host_default); + } else if (strcmp(port_value, host_default)) { + nvme_show_verbose_info( + "config convert: dhchap_key differs between host default and one connection; keeping the connection's own value"); + } +} + +static int convert_port(struct libnvmf_config_emitter *emitter, + const char *hostnqn, const char *hostid, + const char *hostsymname, const char *host_dhchap_key, + const char *subsysnqn, struct json_object *port_obj) +{ + struct libnvmf_params *params; + bool is_dc = json_get_bool(port_obj, "discovery"); + int ret; + + params = libnvmf_params_new(); + if (!params) + return -ENOMEM; + + apply_port_params(params, port_obj); + apply_dhchap_default(params, port_obj, host_dhchap_key); + + ret = libnvmf_config_emit_add(emitter, is_dc, + json_get_string(port_obj, "transport"), + json_get_string(port_obj, "traddr"), + json_get_string(port_obj, "trsvcid"), + subsysnqn, + json_get_string(port_obj, "host_traddr"), + json_get_string(port_obj, "host_iface"), + hostnqn, hostid, params, hostsymname); + + libnvmf_params_free(params); + + /* + * A rejected entry (for example, invalid addressing or a conflicting + * persona) affects only that entry. Log the error and continue + * converting, matching json_parse_port()'s own tolerance. Stop only + * if memory allocation fails. + */ + if (ret == -ENOMEM) + return ret; + if (ret) + nvme_show_error( + "config convert: skipping an entry that could not be added: %s", + libnvme_strerror(-ret)); + + return 0; +} + +static int convert_subsys(struct libnvmf_config_emitter *emitter, + const char *hostnqn, const char *hostid, + const char *hostsymname, const char *host_dhchap_key, + struct json_object *subsys_obj) +{ + struct json_object *port_array; + const char *nqn = json_get_string(subsys_obj, "nqn"); + int p, ret; + + /* The well-known discovery NQN is the emitter's default value. */ + if (nqn && (!*nqn || !strcmp(nqn, NVME_DISC_SUBSYS_NAME))) + nqn = NULL; + + port_array = json_object_object_get(subsys_obj, "ports"); + if (!port_array) + return 0; + + for (p = 0; p < json_object_array_length(port_array); p++) { + struct json_object *port_obj = + json_object_array_get_idx(port_array, p); + + if (!port_obj) + continue; + ret = convert_port(emitter, hostnqn, hostid, hostsymname, + host_dhchap_key, nqn, port_obj); + if (ret) + return ret; + } + + return 0; +} + +static int convert_host(struct libnvmf_config_emitter *emitter, + struct json_object *host_obj) +{ + struct json_object *subsys_array; + const char *hostnqn = json_get_string(host_obj, "hostnqn"); + const char *hostid = json_get_string(host_obj, "hostid"); + const char *hostsymname = json_get_string(host_obj, "hostsymname"); + const char *host_dhchap_key = json_get_string(host_obj, "dhchap_key"); + int s, ret; + + subsys_array = json_object_object_get(host_obj, "subsystems"); + if (!subsys_array) + return 0; + + for (s = 0; s < json_object_array_length(subsys_array); s++) { + struct json_object *subsys_obj = + json_object_array_get_idx(subsys_array, s); + + if (!subsys_obj) + continue; + ret = convert_subsys(emitter, hostnqn, hostid, hostsymname, + host_dhchap_key, subsys_obj); + if (ret) + return ret; + } + + return 0; +} + +int nvme_config_convert_json(struct libnvmf_config_emitter *emitter, + const char *json_file) +{ + struct json_object *json_root, *host_array, *host_obj; + int h, ret; + + json_root = json_object_from_file(json_file); + if (!json_root) { + nvme_show_error("failed to parse %s: %s", json_file, + json_util_get_last_err()); + return -EPROTO; + } + + if (json_object_is_type(json_root, json_type_object)) { + /* Current format: { "hosts": [ ... ] } */ + host_array = json_object_object_get(json_root, "hosts"); + if (!host_array || + !json_object_is_type(host_array, json_type_array)) { + nvme_show_error("%s: expected a 'hosts' array", + json_file); + json_object_put(json_root); + return -EPROTO; + } + } else if (json_object_is_type(json_root, json_type_array)) { + /* Legacy pre-3.0 format: a bare top-level array of hosts. */ + host_array = json_root; + } else { + nvme_show_error("%s: expected a JSON object or array", + json_file); + json_object_put(json_root); + return -EPROTO; + } + + for (h = 0; h < json_object_array_length(host_array); h++) { + host_obj = json_object_array_get_idx(host_array, h); + if (!host_obj) + continue; + ret = convert_host(emitter, host_obj); + if (ret) { + json_object_put(json_root); + return ret; + } + } + + json_object_put(json_root); + + return 0; +} + +#else /* CONFIG_JSONC */ + +int nvme_config_convert_json(struct libnvmf_config_emitter *emitter, + const char *json_file) +{ + nvme_show_error( + "built without json-c; config.json conversion unavailable"); + return -ENOTSUP; +} + +#endif /* CONFIG_JSONC */ diff --git a/config-convert.h b/config-convert.h new file mode 100644 index 0000000000..b87ec53902 --- /dev/null +++ b/config-convert.h @@ -0,0 +1,15 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +#pragma once + +#include + +struct libnvmf_config_emitter; + +/* + * Parse @json_file in the legacy config.json format and add each connection + * to @emitter. This function does not install the generated configuration. + * + * Requires json-c. Returns -ENOTSUP if json-c support is not available. + */ +int nvme_config_convert_json(struct libnvmf_config_emitter *emitter, + const char *json_file); diff --git a/meson.build b/meson.build index 6c856c77c4..1b71e12fd6 100644 --- a/meson.build +++ b/meson.build @@ -588,6 +588,7 @@ if want_nvme if want_fabrics sources += 'fabrics.c' + sources += 'config-convert.c' endif if want_top From caf6764a0244a2b2d61070c1938f38376c44c210 Mon Sep 17 00:00:00 2001 From: Martin Belanger Date: Wed, 15 Jul 2026 15:20:29 -0400 Subject: [PATCH 2/7] fabrics, nvme-cli: add the discovery.conf reader for INI conversion discovery.conf lines are argv-style syntax ("-t rdma -a 192.168.1.4 -s 4420 -q " 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 --- config-convert.c | 124 +++++++++++++++++++++++++++++++++++++++++++++++ config-convert.h | 18 +++++++ fabrics.c | 80 +++++++++++++++++------------- fabrics.h | 46 ++++++++++++++++++ 4 files changed, 234 insertions(+), 34 deletions(-) diff --git a/config-convert.c b/config-convert.c index 5da6a9c2ba..b07d614f8a 100644 --- a/config-convert.c +++ b/config-convert.c @@ -6,13 +6,16 @@ */ #include +#include #include #include #include "common.h" #include "config-convert.h" +#include "fabrics.h" #include "nvme-print.h" +#include "util/cleanup.h" #ifdef CONFIG_JSONC #include @@ -295,3 +298,124 @@ int nvme_config_convert_json(struct libnvmf_config_emitter *emitter, } #endif /* CONFIG_JSONC */ + +static void add_tunable_params(struct libnvmf_params *params, + const struct nvmf_args *fa) +{ + char buf[32]; + + if (fa->nr_io_queues) { + snprintf(buf, sizeof(buf), "%d", fa->nr_io_queues); + libnvmf_params_set(params, "nr-io-queues", buf); + } + if (fa->nr_write_queues) { + snprintf(buf, sizeof(buf), "%d", fa->nr_write_queues); + libnvmf_params_set(params, "nr-write-queues", buf); + } + if (fa->nr_poll_queues) { + snprintf(buf, sizeof(buf), "%d", fa->nr_poll_queues); + libnvmf_params_set(params, "nr-poll-queues", buf); + } + if (fa->queue_size) { + snprintf(buf, sizeof(buf), "%d", fa->queue_size); + libnvmf_params_set(params, "queue-size", buf); + } + if (fa->keep_alive_tmo) { + snprintf(buf, sizeof(buf), "%d", fa->keep_alive_tmo); + libnvmf_params_set(params, "keep-alive-tmo", buf); + } + if (fa->reconnect_delay) { + snprintf(buf, sizeof(buf), "%d", fa->reconnect_delay); + libnvmf_params_set(params, "reconnect-delay", buf); + } + if (fa->ctrl_loss_tmo != NVMF_DEF_CTRL_LOSS_TMO) { + snprintf(buf, sizeof(buf), "%d", fa->ctrl_loss_tmo); + libnvmf_params_set(params, "ctrl-loss-tmo", buf); + } + if (fa->fast_io_fail_tmo) { + snprintf(buf, sizeof(buf), "%d", fa->fast_io_fail_tmo); + libnvmf_params_set(params, "fast-io-fail-tmo", buf); + } + if (fa->tos != -1) { + snprintf(buf, sizeof(buf), "%d", fa->tos); + libnvmf_params_set(params, "tos", buf); + } + if (fa->duplicate_connect) + libnvmf_params_set(params, "duplicate-connect", "true"); + if (fa->disable_sqflow) + libnvmf_params_set(params, "disable-sqflow", "true"); + if (fa->hdr_digest) + libnvmf_params_set(params, "hdr-digest", "true"); + if (fa->data_digest) + libnvmf_params_set(params, "data-digest", "true"); + if (fa->tls) + libnvmf_params_set(params, "tls", "true"); + if (fa->concat) + libnvmf_params_set(params, "concat", "true"); + if (fa->hostkey) + libnvmf_params_set(params, "dhchap-secret", fa->hostkey); + if (fa->ctrlkey) + libnvmf_params_set(params, "dhchap-ctrl-secret", fa->ctrlkey); + if (fa->keyring) + libnvmf_params_set(params, "keyring", fa->keyring); + if (fa->tls_key) + libnvmf_params_set(params, "tls-key", fa->tls_key); + if (fa->tls_key_identity) + libnvmf_params_set(params, "tls-key-identity", + fa->tls_key_identity); +} + +int nvme_config_convert_discovery_args(struct libnvmf_config_emitter *emitter, + const struct nvmf_args *fa) +{ + struct libnvmf_params *params; + int ret; + + params = libnvmf_params_new(); + if (!params) + return -ENOMEM; + + add_tunable_params(params, fa); + + ret = libnvmf_config_emit_add(emitter, true, fa->transport, fa->traddr, + fa->trsvcid, fa->subsysnqn, fa->host_traddr, + fa->host_iface, fa->hostnqn, fa->hostid, params, NULL); + + libnvmf_params_free(params); + + /* + * A rejected entry affects only that entry. Log the error and + * continue converting. Stop only if memory allocation fails. + */ + if (ret == -ENOMEM) + return ret; + if (ret) + nvme_show_error( + "discovery.conf: skipping a line that could not be added: %s", + libnvme_strerror(-ret)); + + return 0; +} + +int nvme_config_convert_discovery(struct libnvmf_config_emitter *emitter, + const char *disc_file) +{ + __cleanup_file FILE *f = NULL; + static char line[4096]; + int ret; + + f = fopen(disc_file, "r"); + if (!f) { + nvme_show_error("failed to open %s: %s", disc_file, + strerror(errno)); + return -errno; + } + + while (fgets(line, sizeof(line), f)) { + ret = nvmf_convert_discovery_line(emitter, line); + if (ret) + return ret; + } + + return 0; +} diff --git a/config-convert.h b/config-convert.h index b87ec53902..98b94487a0 100644 --- a/config-convert.h +++ b/config-convert.h @@ -4,6 +4,7 @@ #include struct libnvmf_config_emitter; +struct nvmf_args; /* * Parse @json_file in the legacy config.json format and add each connection @@ -13,3 +14,20 @@ struct libnvmf_config_emitter; */ int nvme_config_convert_json(struct libnvmf_config_emitter *emitter, const char *json_file); + +/* + * Parse @disc_file in the legacy discovery.conf format and add each entry + * to @emitter. This function does not install the generated configuration. + * Malformed entries are logged and skipped. + */ +int nvme_config_convert_discovery(struct libnvmf_config_emitter *emitter, + const char *disc_file); + +/* + * Add one parsed discovery.conf entry to @emitter. + * + * The caller is responsible for parsing the input line into @fa. Invalid + * entries are logged and skipped. Only -ENOMEM is treated as a fatal error. + */ +int nvme_config_convert_discovery_args(struct libnvmf_config_emitter *emitter, + const struct nvmf_args *fa); diff --git a/fabrics.c b/fabrics.c index 680b3b29b8..7430f46f29 100644 --- a/fabrics.c +++ b/fabrics.c @@ -50,6 +50,7 @@ #endif #include "common.h" +#include "config-convert.h" #include "nvme.h" #include "nvme-print.h" #include "fabrics.h" @@ -102,40 +103,6 @@ static const char *nvmf_tls = "enable TLS"; static const char *nvmf_concat = "enable secure concatenation"; static const char *nvmf_config_file = "Use specified JSON configuration file or 'none' to disable"; -struct nvmf_args { - const char *subsysnqn; - const char *transport; - const char *traddr; - const char *host_traddr; - const char *host_iface; - const char *trsvcid; - const char *hostnqn; - const char *hostid; - const char *hostkey; - const char *ctrlkey; - const char *keyring; - const char *tls_key; - const char *tls_key_identity; - int queue_size; - int nr_io_queues; - int reconnect_delay; - int ctrl_loss_tmo; - int fast_io_fail_tmo; - int keep_alive_tmo; - int nr_write_queues; - int nr_poll_queues; - int tos; - long keyring_id; - long tls_key_id; - long tls_configured_key_id; - bool duplicate_connect; - bool disable_sqflow; - bool hdr_digest; - bool data_digest; - bool tls; - bool concat; -}; - #define NVMF_ARGS(n, f, ...) \ NVME_ARGS(n, \ OPT_STRING("transport", 't', "STR", &f.transport, nvmf_tport), \ @@ -479,6 +446,51 @@ static int hook_parser_next_line(struct libnvmf_context *fctx, void *user_data) return 0; } +/* + * Parse one discovery.conf line -- the same argv-style syntax 'nvme + * discover'/'connect-all' accept, reusing NVMF_ARGS so every short and long + * form works identically to real usage -- and hand the parsed arguments to + * nvme_config_convert_discovery_args(). @line is modified in place (strsep()). + * + * A blank line, a comment, or one with neither transport nor traddr is + * silently skipped (0, nothing added): matches hook_parser_next_line()'s + * own tolerance for a discovery.conf that mixes real entries with commentary. + */ +int nvmf_convert_discovery_line(struct libnvmf_config_emitter *emitter, + char *line) +{ + struct nvmf_args fa = { 0 }; + char *argv[MAX_DISC_ARGS] = { "discovery.conf" }; + char *ptr, *p = line; + int argc = 1; + bool line_persistent = false, line_force = false; + + NVMF_ARGS(opts, fa, + OPT_FLAG("persistent", 'p', &line_persistent, + "persistent discovery connection"), + OPT_FLAG("force", 0, &line_force, + "Force persistent discovery controller creation")); + + if (line[0] == '#' || line[0] == '\n' || line[0] == '\0') + return 0; + + fa.tos = -1; + fa.ctrl_loss_tmo = NVMF_DEF_CTRL_LOSS_TMO; + + while ((ptr = strsep(&p, " =\n")) != NULL && argc < MAX_DISC_ARGS - 1) + argv[argc++] = ptr; + argv[argc] = NULL; + + fa.subsysnqn = NVME_DISC_SUBSYS_NAME; + if (argconfig_parse(argc, argv, "discovery.conf", opts)) + return 0; + + if (!fa.transport && !fa.traddr) + return 0; + + return nvme_config_convert_discovery_args(emitter, &fa); +} + static int setup_common_context(struct libnvmf_context *fctx, struct nvmf_args *fa) { diff --git a/fabrics.h b/fabrics.h index f8b409dcef..12607500b4 100644 --- a/fabrics.h +++ b/fabrics.h @@ -1,9 +1,55 @@ /* SPDX-License-Identifier: GPL-2.0-or-later */ #pragma once +/* Parsed "nvme connect"/"discover"/"connect-all" argv-style arguments. */ +struct nvmf_args { + const char *subsysnqn; + const char *transport; + const char *traddr; + const char *host_traddr; + const char *host_iface; + const char *trsvcid; + const char *hostnqn; + const char *hostid; + const char *hostkey; + const char *ctrlkey; + const char *keyring; + const char *tls_key; + const char *tls_key_identity; + int queue_size; + int nr_io_queues; + int reconnect_delay; + int ctrl_loss_tmo; + int fast_io_fail_tmo; + int keep_alive_tmo; + int nr_write_queues; + int nr_poll_queues; + int tos; + long keyring_id; + long tls_key_id; + long tls_configured_key_id; + bool duplicate_connect; + bool disable_sqflow; + bool hdr_digest; + bool data_digest; + bool tls; + bool concat; +}; + +struct libnvmf_config_emitter; + int fabrics_discovery(const char *desc, int argc, char **argv, bool connect); int fabrics_connect(const char *desc, int argc, char **argv); int fabrics_disconnect(const char *desc, int argc, char **argv); int fabrics_disconnect_all(const char *desc, int argc, char **argv); int fabrics_config(const char *desc, int argc, char **argv); int fabrics_dim(const char *desc, int argc, char **argv); + +/* + * Parse one discovery.conf line (the argv-style syntax 'nvme discover' + * accepts) and add it to @emitter as a discovery controller entry. @line is + * modified in place. Returns 0 for a parsed entry, a skipped blank/comment + * line, or a malformed one; negative errno only on allocation failure. + */ +int nvmf_convert_discovery_line(struct libnvmf_config_emitter *emitter, + char *line); From 24b76a4010f55a2fa3c9362f44d32c27c5fa2b5d Mon Sep 17 00:00:00 2001 From: Martin Belanger Date: Wed, 15 Jul 2026 17:14:03 -0400 Subject: [PATCH 3/7] nvme-cli: add the config-convert command 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 .converted. A rerun against a source already converted on a prior run (renamed to .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 --- config-convert.c | 150 +++++++++++++++++++++++++++++++++++++++++++++++ config-convert.h | 12 ++++ fabrics.c | 2 - fabrics.h | 15 ++++- meson.build | 6 +- nvme-builtin.h | 1 + nvme.c | 9 +++ 7 files changed, 189 insertions(+), 6 deletions(-) diff --git a/config-convert.c b/config-convert.c index b07d614f8a..a832a13548 100644 --- a/config-convert.c +++ b/config-convert.c @@ -8,13 +8,16 @@ #include #include #include +#include #include #include "common.h" #include "config-convert.h" #include "fabrics.h" +#include "logging.h" #include "nvme-print.h" +#include "util/argconfig.h" #include "util/cleanup.h" #ifdef CONFIG_JSONC @@ -419,3 +422,150 @@ int nvme_config_convert_discovery(struct libnvmf_config_emitter *emitter, return 0; } + +/* Best effort. The configuration has already been installed. */ +static void rename_to_converted(const char *path) +{ + __cleanup_free char *dst = NULL; + + if (asprintf(&dst, "%s.converted", path) < 0) + return; + + if (rename(path, dst)) + nvme_show_error( + "converted %s but failed to rename it to %s: %s", + path, dst, strerror(errno)); +} + +/* True if @path is gone because a prior run already renamed it away. */ +static bool already_converted(const char *path) +{ + __cleanup_free char *converted = NULL; + + if (asprintf(&converted, "%s.converted", path) < 0) + return false; + + return !access(converted, F_OK); +} + +static int install_converted(struct libnvmf_config_emitter *emitter, + const char *output_file, const char *json_path, + const char *disc_path, bool converted_json, + bool converted_disc, bool force) +{ + int ret; + + ret = libnvmf_config_emit_install(emitter, output_file, force); + if (ret == -EEXIST) { + nvme_show_error("%s already exists; refusing to overwrite", + output_file); + return ret; + } + if (ret) { + nvme_show_error("failed to write %s: %s", output_file, + libnvme_strerror(-ret)); + return ret; + } + + if (converted_json) + rename_to_converted(json_path); + if (converted_disc) + rename_to_converted(disc_path); + + return 0; +} + +int nvme_config_convert(const char *desc, int argc, char **argv) +{ + char *config_file = NULL; + char *output_file = NULL; + const char *target; + const char *json_path; + bool verbose = false; + bool force = false; + bool converted_json = false, converted_disc = false; + bool json_already_done = false, disc_already_done = false; + __cleanup_nvme_global_ctx struct libnvme_global_ctx *ctx = NULL; + struct libnvmf_config_emitter *emitter = NULL; + int ret; + + OPT_ARGS(opts) = { + OPT_STRING("config", 'J', "FILE", &config_file, + "convert this JSON file (default: config.json)"), + OPT_STRING("output", 'o', "FILE", &output_file, + "write result here (default: nvme-fabrics.conf)"), + OPT_FLAG("force", 0, &force, + "overwrite an existing target"), + OPT_FLAG("verbose", 'v', &verbose, "increase output verbosity"), + OPT_END() + }; + + ret = argconfig_parse(argc, argv, desc, opts); + if (ret) + return ret; + + log_level = map_log_level(verbose ? 1 : 0, false); + + ret = nvme_create_global_ctx(&ctx); + if (ret) { + nvme_show_error("Failed to create topology root: %s", + libnvme_strerror(-ret)); + return ret; + } + libnvme_set_logging_level(ctx, log_level, false, false); + + emitter = libnvmf_config_emit_new(ctx); + if (!emitter) + return -ENOMEM; + + json_path = config_file ? config_file : PATH_NVMF_CONFIG; + if (!access(json_path, F_OK)) { + ret = nvme_config_convert_json(emitter, json_path); + if (ret) + goto out; + converted_json = true; + } else if (already_converted(json_path)) { + json_already_done = true; + } else if (config_file) { + /* + * An explicit --config to a file that neither exists nor was + * ever converted: let the JSON parser produce its own + * "failed to parse" error instead of silently no-op'ing, + * since this was an explicit ask. + */ + ret = nvme_config_convert_json(emitter, json_path); + if (ret) + goto out; + converted_json = true; + } + + if (!access(PATH_NVMF_DISC, F_OK)) { + ret = nvme_config_convert_discovery(emitter, PATH_NVMF_DISC); + if (ret) + goto out; + converted_disc = true; + } else if (already_converted(PATH_NVMF_DISC)) { + disc_already_done = true; + } + + if (!converted_json && !converted_disc) { + if (json_already_done || disc_already_done) { + nvme_show_result("already converted; nothing to do"); + ret = 0; + goto out; + } + nvme_show_error("nothing to convert: neither %s nor %s exists", + json_path, PATH_NVMF_DISC); + ret = -ENOENT; + goto out; + } + + target = output_file ? output_file : PATH_NVMF_INI; + ret = install_converted(emitter, target, json_path, PATH_NVMF_DISC, + converted_json, converted_disc, force); + +out: + libnvmf_config_emit_free(emitter); + + return ret; +} diff --git a/config-convert.h b/config-convert.h index 98b94487a0..57e5911bb6 100644 --- a/config-convert.h +++ b/config-convert.h @@ -31,3 +31,15 @@ int nvme_config_convert_discovery(struct libnvmf_config_emitter *emitter, */ int nvme_config_convert_discovery_args(struct libnvmf_config_emitter *emitter, const struct nvmf_args *fa); + +/* + * Implement the "nvme config-convert" command. + * + * Convert the legacy config.json and/or discovery.conf configuration files + * to the INI format, matching "nvme connect-all"'s own default behavior: + * discovery.conf is always converted when it exists, regardless of whether + * --config points at a non-default config.json. Write the result to + * --output or, if omitted, to the default location. Refuse to overwrite an + * existing target unless --force is used. + */ +int nvme_config_convert(const char *desc, int argc, char **argv); diff --git a/fabrics.c b/fabrics.c index 7430f46f29..a0c368911c 100644 --- a/fabrics.c +++ b/fabrics.c @@ -58,8 +58,6 @@ #include "logging.h" #include "util/sighdl.h" -#define PATH_NVMF_DISC SYSCONFDIR "/nvme/discovery.conf" -#define PATH_NVMF_CONFIG SYSCONFDIR "/nvme/config.json" #define PATH_NVMF_RUNDIR RUNDIR "/nvme" #define MAX_DISC_ARGS 32 #define MAX_DISC_RETRIES 10 diff --git a/fabrics.h b/fabrics.h index 12607500b4..9ba2c8cfbb 100644 --- a/fabrics.h +++ b/fabrics.h @@ -36,8 +36,6 @@ struct nvmf_args { bool concat; }; -struct libnvmf_config_emitter; - int fabrics_discovery(const char *desc, int argc, char **argv, bool connect); int fabrics_connect(const char *desc, int argc, char **argv); int fabrics_disconnect(const char *desc, int argc, char **argv); @@ -45,6 +43,19 @@ int fabrics_disconnect_all(const char *desc, int argc, char **argv); int fabrics_config(const char *desc, int argc, char **argv); int fabrics_dim(const char *desc, int argc, char **argv); +/* + * Legacy config.json/discovery.conf support -- both the explicit converter + * ("nvme config-convert", config-convert.c/.h) and the implicit fallback + * (fabrics_discovery()/fabrics_config() reading these files directly) go + * away together when legacy config support is eventually dropped; this + * section (and nvmf_convert_discovery_line() in fabrics.c) goes with them. + */ +struct libnvmf_config_emitter; + +#define PATH_NVMF_DISC SYSCONFDIR "/nvme/discovery.conf" +#define PATH_NVMF_CONFIG SYSCONFDIR "/nvme/config.json" +#define PATH_NVMF_INI SYSCONFDIR "/nvme/nvme-fabrics.conf" + /* * Parse one discovery.conf line (the argv-style syntax 'nvme discover' * accepts) and add it to @emitter as a discovery controller entry. @line is diff --git a/meson.build b/meson.build index 1b71e12fd6..224e79e0a2 100644 --- a/meson.build +++ b/meson.build @@ -587,8 +587,10 @@ if want_nvme endif if want_fabrics - sources += 'fabrics.c' - sources += 'config-convert.c' + sources += [ + 'fabrics.c', + 'config-convert.c', + ] endif if want_top diff --git a/nvme-builtin.h b/nvme-builtin.h index b40c6b4446..5512a833b4 100644 --- a/nvme-builtin.h +++ b/nvme-builtin.h @@ -113,6 +113,7 @@ COMMAND_LIST( ENTRY("disconnect", "Disconnect from NVMeoF subsystem", disconnect_cmd) ENTRY("disconnect-all", "Disconnect from all connected NVMeoF subsystems", disconnect_all_cmd) ENTRY("config", "Configuration of NVMeoF subsystems", config_cmd) + ENTRY("config-convert", "Convert config to INI", config_convert_cmd) ENTRY("dim", "Send Discovery Information Management command to a Discovery Controller", dim_cmd) ENTRY("gen-hostnqn", "Generate NVMeoF host NQN", gen_hostnqn_cmd) ENTRY("show-hostnqn", "Show NVMeoF host NQN", show_hostnqn_cmd) diff --git a/nvme.c b/nvme.c index c00d1f12d9..dfd21cdeb3 100644 --- a/nvme.c +++ b/nvme.c @@ -48,6 +48,7 @@ #include #include "common.h" +#include "config-convert.h" #include "fabrics.h" #include "logging.h" #include "nvme-cmds.h" @@ -10846,6 +10847,14 @@ static int config_cmd(int argc, char **argv, struct command *acmd, struct plugin return fabrics_config(desc, argc, argv); } +static int config_convert_cmd(int argc, char **argv, struct command *acmd, + struct plugin *plugin) +{ + const char *desc = "Convert config.json/discovery.conf to INI"; + + return nvme_config_convert(desc, argc, argv); +} + static int dim_cmd(int argc, char **argv, struct command *acmd, struct plugin *plugin) { const char *desc = From c2af72554f263969f01aee84ee40912a528cc808 Mon Sep 17 00:00:00 2001 From: Martin Belanger Date: Wed, 15 Jul 2026 17:14:09 -0400 Subject: [PATCH 4/7] libnvme/design: fix stale config-convert references in CONFIG.md 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 .converted rather than left in place. Signed-off-by: Martin Belanger --- libnvme/design/CONFIG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libnvme/design/CONFIG.md b/libnvme/design/CONFIG.md index 38960d90ec..f6ed472382 100644 --- a/libnvme/design/CONFIG.md +++ b/libnvme/design/CONFIG.md @@ -59,7 +59,7 @@ The drop-in directory name is derived mechanically from whatever file you point - `--config /etc/nvme/nvme-fabrics.conf` → reads that file **and** `/etc/nvme/nvme-fabrics.conf.d/`. - `--config /home/bob/nvme-for-bob.conf` → reads that file **and** `/home/bob/nvme-for-bob.conf.d/` — rerooted to any base directory for free. - Omitting `--config` → the default `/etc/nvme/nvme-fabrics.conf` (+ its `.d/`). -- `--config /etc/nvme/config.json` (a legacy JSON file, recognized by its `.json` extension) → a hard error naming `nvme config convert`; libnvme's reader never reads JSON. +- `--config /etc/nvme/config.json` (a legacy JSON file, recognized by its `.json` extension) → a hard error naming `nvme config-convert`; libnvme's reader never reads JSON. Because the config is reached by an explicit path (`--config FILE`, or `libnvmf_config_read(ctx, file)` at the API level), unit testing is straightforward: point at a throwaway file under `/tmp` (and its derived `.d/`) instead of the default `/etc/nvme/nvme-fabrics.conf`. @@ -357,7 +357,7 @@ Specification documents this design cites (section numbers refer to the revision nvme-cli has historically stored saved connections in `/etc/nvme/config.json`, via the optional `json-c` library. That format is superseded by the INI format described here: it removes libnvme's dependency on `json-c`, it supports comments (JSON does not), and it matches the hand-edit style of systemd's own `.conf` / `.network` / `.service` files. -libnvme itself never reads `config.json` — INI is the only format it understands. The transition is staged entirely in nvme-cli, above the library: `nvme config convert` reads the legacy `config.json` / `discovery.conf` once and writes the INI equivalent; the legacy files are left in place, untouched, but never read again. Routine operation only reads the INI — the tool never rewrites it. +libnvme itself never reads `config.json` — INI is the only format it understands. The transition is staged entirely in nvme-cli, above the library: `nvme config-convert` reads the legacy `config.json` / `discovery.conf` once and writes the INI equivalent, then renames each converted legacy file to `.converted` so a repeat run does not read it again. Routine operation only reads the INI — the tool never rewrites it. `discovery.conf` is also superseded. By its own man page it is "a list of connect-all commands to run": limited to Discovery Controllers by construction, with no way to express an I/O Controller or global defaults, and no `[Host]`-style persona (the hostnqn/hostid pair has to be repeated on every line for one identity, with nothing validating the result). The format here covers the full set, so it replaces `discovery.conf` too. From ab7e967f85808602e97e53da1c9cc47bfe342bfb Mon Sep 17 00:00:00 2001 From: Martin Belanger Date: Wed, 15 Jul 2026 17:14:12 -0400 Subject: [PATCH 5/7] doc: add nvme-config-convert man page Signed-off-by: Martin Belanger --- Documentation/cmds-main.txt | 3 + Documentation/meson.build | 1 + Documentation/nvme-config-convert.txt | 95 +++++++++++++++++++++++++++ 3 files changed, 99 insertions(+) create mode 100644 Documentation/nvme-config-convert.txt diff --git a/Documentation/cmds-main.txt b/Documentation/cmds-main.txt index 14a6ff6998..4b70ef8e7f 100644 --- a/Documentation/cmds-main.txt +++ b/Documentation/cmds-main.txt @@ -169,6 +169,9 @@ linknvme:nvme-disconnect[1]:: linknvme:nvme-disconnect-all[1]:: Disconnect from all NVMe-over-Fabrics subsystems +linknvme:nvme-config-convert[1]:: + Convert a legacy NVMe-oF configuration to the INI format + linknvme:nvme-get-property[1]:: Reads and shows NVMe-over-Fabrics controller property diff --git a/Documentation/meson.build b/Documentation/meson.build index a2e9a240af..c48a1f00bc 100644 --- a/Documentation/meson.build +++ b/Documentation/meson.build @@ -10,6 +10,7 @@ adoc_sources = [ 'nvme-changed-ns-list-log', 'nvme-cmdset-ind-id-ns', 'nvme-compare', + 'nvme-config-convert', 'nvme-connect', 'nvme-connect-all', 'nvme-copy', diff --git a/Documentation/nvme-config-convert.txt b/Documentation/nvme-config-convert.txt new file mode 100644 index 0000000000..c17211fc08 --- /dev/null +++ b/Documentation/nvme-config-convert.txt @@ -0,0 +1,95 @@ +nvme-config-convert(1) +====================== + +NAME +---- +nvme-config-convert - Convert a legacy NVMe-oF configuration to the INI format + +SYNOPSIS +-------- +[verse] +'nvme' 'config-convert' + [--config= | -J ] + [--output= | -o ] + [--force] + [--verbose | -v] + +DESCRIPTION +----------- +Converts a pre-3.0 NVMe-oF configuration from the legacy 'config.json' +JSON store and/or the 'discovery.conf' argv-style file to the INI +configuration format used by 'nvme connect-all' and 'nvme discover'. The +INI format is documented at +https://github.com/linux-nvme/nvme-cli/blob/master/libnvme/design/CONFIG.md + +By default, the command looks for the legacy files +@SYSCONFDIR@/nvme/config.json and @SYSCONFDIR@/nvme/discovery.conf and +converts any that exist into a single configuration, matching the behavior +of 'nvme connect-all'. Use --config to convert a config.json at a +non-default path instead; as with 'nvme connect-all', discovery.conf +conversion is not gated by --config — @SYSCONFDIR@/nvme/discovery.conf is +always converted when it exists, whether or not --config is given. + +The converted configuration is written to the file specified by +--output. If --output is not specified, the default location, +@SYSCONFDIR@/nvme/nvme-fabrics.conf, is used. Unless --force is specified, +the command refuses to overwrite an existing configuration. On success, +the converted legacy file(s) are renamed to '.converted' to prevent +conversion on subsequent runs. Running the command again is safe: a source +file that is missing only because a prior run already converted and +renamed it is treated as already done, not as an error. + +Malformed entries (for example, invalid addressing or an unreadable port) +are skipped with a log message. Conversion continues unless a memory +allocation failure occurs. + +OPTIONS +------- +-J :: +--config=:: + Convert this JSON file instead of the default + @SYSCONFDIR@/nvme/config.json. + +-o :: +--output=:: + Write the converted configuration here instead of the default + @SYSCONFDIR@/nvme/nvme-fabrics.conf. + +--force:: + Overwrite an existing configuration at the target. Any manual changes + are lost. + +-v:: +--verbose:: + Increase the level of detail in the output. + +EXAMPLES +-------- +* Convert the system's legacy configuration in place: ++ +------------ +# nvme config-convert +------------ + +* Convert one specific configuration file for testing, without touching + the system configuration: ++ +------------ +# nvme config-convert --config /tmp/config.json --output /tmp/nvme-fabrics.conf +------------ ++ +Note: unlike config.json, discovery.conf has no --config-style override — +its path is always @SYSCONFDIR@/nvme/discovery.conf. If that file exists on +the system, this command still converts it and renames it to +'discovery.conf.converted', even though --config points elsewhere. + +SEE ALSO +-------- +nvme-config(1) +nvme-connect-all(1) +nvme-discover(1) +https://github.com/linux-nvme/nvme-cli/blob/master/libnvme/design/CONFIG.md + +NVME +---- +Part of the nvme-user suite From 41426365e34e2037a9cffd53ae571a2ba09a7fc0 Mon Sep 17 00:00:00 2001 From: Martin Belanger Date: Fri, 17 Jul 2026 10:25:04 -0400 Subject: [PATCH 6/7] tests: extract shared command/logging infra into TestNVMeBase 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 --- tests/nvme_test.py | 61 +++++++++++++++++++++++++++++----------------- 1 file changed, 38 insertions(+), 23 deletions(-) diff --git a/tests/nvme_test.py b/tests/nvme_test.py index dc4b9a9996..86fc16dbde 100644 --- a/tests/nvme_test.py +++ b/tests/nvme_test.py @@ -55,7 +55,43 @@ def to_decimal(value): return val -class TestNVMe(unittest.TestCase): +class TestNVMeBase(unittest.TestCase): + + """ + Shared command-execution and logging setup for nvme-cli Python tests. + Requires no real device -- see TestNVMe below for the hardware-backed + subclass that layers device setup, controller/namespace discovery, and + PCI validation on top. + """ + + def setUp(self): + self.nvme_bin = "nvme" + if not logging.getLogger().handlers: + logging.basicConfig(format='%(message)s', stream=sys.stdout) + + def run_cmd(self, cmd, stdin_data=None, shell=True): + """ Run a command using subprocess.run, log the command and its + output, and return the CompletedProcess result. + - Args: + - cmd : shell command string to execute when shell=True + (the default); an argv list when shell=False. + - stdin_data : optional string to pass as stdin input. + - shell : passed straight through to subprocess.run(). + - Returns: + - CompletedProcess result. + """ + logger.debug(f"Running: {cmd}") + result = subprocess.run(cmd, shell=shell, stdout=subprocess.PIPE, + stderr=subprocess.PIPE, encoding='utf-8', + input=stdin_data) + if result.stdout: + logger.debug(result.stdout) + if result.stderr: + logger.debug(result.stderr) + return result + + +class TestNVMe(TestNVMeBase): """ Represents a testcase, each testcase should inherit this @@ -76,11 +112,11 @@ def is_windows(self): def setUp(self): """ Pre Section for TestNVMe. """ + super().setUp() # common code used in various testcases. self.ctrl = "XXX" self.ns1 = "XXX" self.test_log_dir = "XXX" - self.nvme_bin = "nvme" self.do_validate_pci_device = True self.default_nsid = 0x1 self.flbas = 0 @@ -173,8 +209,6 @@ def load_config(self): log_level_str = config.get('log_level', 'DEBUG' if config.get('debug', False) else 'WARNING') log_level = getattr(logging, log_level_str.upper(), logging.WARNING) - if not logging.getLogger().handlers: - logging.basicConfig(format='%(message)s', stream=sys.stdout) logging.getLogger().setLevel(log_level) logger.debug("Using nvme binary '%s'", self.nvme_bin) @@ -197,25 +231,6 @@ def setup_log_dir(self, test_name): sys.stdout = TestNVMeLogger(self.test_log_dir + "/" + "stdout.log") sys.stderr = TestNVMeLogger(self.test_log_dir + "/" + "stderr.log") - def run_cmd(self, cmd, stdin_data=None): - """ Run a shell command using subprocess.run, log the command and its - output, and return the CompletedProcess result. - - Args: - - cmd : shell command string to execute. - - stdin_data : optional string to pass as stdin input. - - Returns: - - CompletedProcess result. - """ - logger.debug(f"Running: {cmd}") - result = subprocess.run(cmd, shell=True, stdout=subprocess.PIPE, - stderr=subprocess.PIPE, encoding='utf-8', - input=stdin_data) - if result.stdout: - logger.debug(result.stdout) - if result.stderr: - logger.debug(result.stderr) - return result - def parse_json_output(self, output, context, expected_type=dict): """Parse JSON output and fail test clearly on malformed or wrong-typed data. From 4f377fbd68c406fbcd7dde39759498b9bebc6416 Mon Sep 17 00:00:00 2001 From: Martin Belanger Date: Fri, 17 Jul 2026 10:25:10 -0400 Subject: [PATCH 7/7] tests: add end-to-end coverage for nvme config-convert 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 --- tests/meson.build | 13 ++ tests/nvme_config_convert_test.py | 276 ++++++++++++++++++++++++++++++ 2 files changed, 289 insertions(+) create mode 100644 tests/nvme_config_convert_test.py diff --git a/tests/meson.build b/tests/meson.build index a2171ac4ee..8f5401755f 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -75,6 +75,19 @@ if want_fabrics and 'registry' in selected_plugins endif endif +# Standalone CLI end-to-end test for "nvme config-convert". Needs no device; +# uses an explicit --config/--output pair under /tmp instead. Requires +# json-c since the legacy config.json reader is a no-op without it. +if want_fabrics and json_c_dep.found() + _py3 = find_program('python3', required: false) + if _py3.found() + test('nvme-cli - config-convert-cli', + _py3, + args: [files('nvme_config_convert_test.py'), nvme_exe], + depends: nvme_exe) + endif +endif + mypy = find_program( 'mypy', required : false, diff --git a/tests/nvme_config_convert_test.py b/tests/nvme_config_convert_test.py new file mode 100644 index 0000000000..906c59ab1d --- /dev/null +++ b/tests/nvme_config_convert_test.py @@ -0,0 +1,276 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: GPL-2.0-or-later +# +# This file is part of nvme-cli. +# Copyright (c) 2026 Dell Technologies Inc. or its subsidiaries. +# +# Authors: Martin Belanger +"""CLI integration tests for 'nvme config-convert'. + +Tests invoke the nvme binary directly with an explicit --config file and +--output under a temporary directory, so no real hardware is needed and no +system file is touched by the --config/--output path itself. + +discovery.conf conversion is deliberately NOT exercised here: its path is +hardcoded to SYSCONFDIR/nvme/discovery.conf with no override (--config only +ever affects config.json), so it cannot be sandboxed the way --config/ +--output can. Its parsing logic (nvme_config_convert_discovery()) should be +covered separately, e.g. as a C unit test that calls it directly against a +temp file. Because the real discovery.conf is always converted (and renamed) +when it exists, this whole suite skips itself if one is present on the +machine running the tests -- a config-convert invocation must never rename a +developer's real discovery.conf as a side effect of running the test suite. + +Usage: python3 nvme_config_convert_test.py +""" +import glob +import json +import os +import shutil +import sys +import tempfile +import unittest + +from nvme_test import TestNVMeBase + + +# Capture the nvme binary path from argv before unittest.main() strips it. +_NVME_BIN = sys.argv[1] \ + if len(sys.argv) > 1 and not sys.argv[1].startswith('-') \ + else 'nvme' + +_REAL_DISCOVERY_CONF = '/etc/nvme/discovery.conf' + + +@unittest.skipIf(os.path.exists(_REAL_DISCOVERY_CONF), + f'{_REAL_DISCOVERY_CONF} exists on this machine; ' + 'config-convert always converts and renames it, so ' + 'skipping to avoid touching real system configuration') +class ConfigConvertCLITest(TestNVMeBase): + + def setUp(self): + super().setUp() + self.nvme_bin = _NVME_BIN + self.tmpdir = tempfile.mkdtemp(prefix='nvme-config-convert-test-', + dir='/tmp') + self.config_json = os.path.join(self.tmpdir, 'config.json') + self.output_ini = os.path.join(self.tmpdir, 'nvme-fabrics.conf') + + def tearDown(self): + shutil.rmtree(self.tmpdir) + + def _write_json(self, obj): + with open(self.config_json, 'w') as f: + json.dump(obj, f) + + def _run(self, *args, expect_fail=False): + cmd = [self.nvme_bin] + list(args) + result = self.run_cmd(cmd, shell=False) + if expect_fail: + self.assertNotEqual(result.returncode, 0, + f'Command {cmd} unexpectedly succeeded:\n' + f'{result.stdout}') + else: + self.assertEqual(result.returncode, 0, + f'Command {cmd} failed:\n{result.stderr}') + return result + + def _convert(self, *extra_args, expect_fail=False): + return self._run('config-convert', '--config', self.config_json, + '--output', self.output_ini, *extra_args, + expect_fail=expect_fail) + + def _read_output(self): + # A persona with an explicit hostnqn/hostid lands in its own + # .d/NNN-persona.conf drop-in, not the main file; only the + # default (no explicit identity) persona is written to the main + # file itself. Concatenate both so assertions don't care which. + chunks = [] + if os.path.exists(self.output_ini): + with open(self.output_ini) as f: + chunks.append(f.read()) + for dropin in sorted(glob.glob(self.output_ini + '.d/*.conf')): + with open(dropin) as f: + chunks.append(f.read()) + return '\n'.join(chunks) + + # ------------------------------------------------------------------ # + # happy path # + # ------------------------------------------------------------------ # + + def test_convert_dc_and_subsystem(self): + self._write_json({ + 'hosts': [{ + 'hostnqn': 'nqn.2014-08.org.nvmexpress:uuid:1111', + 'hostid': '46ba5037-7ce5-41fa-9452-48477bf00080', + 'hostsymname': 'lab-host-01', + 'subsystems': [ + { + 'nqn': 'nqn.2014-08.org.nvmexpress.discovery', + 'ports': [{ + 'transport': 'tcp', + 'traddr': '192.168.1.10', + 'trsvcid': '8009', + 'discovery': True, + }], + }, + { + 'nqn': 'nqn.2024-01.com.example:data.vol1', + 'ports': [{ + 'transport': 'tcp', + 'traddr': '192.168.1.20', + 'trsvcid': '4420', + 'nr_io_queues': 4, + }], + }, + ], + }], + }) + self._convert() + content = self._read_output() + self.assertIn('[Host]', content) + self.assertIn('hostnqn = nqn.2014-08.org.nvmexpress:uuid:1111', + content) + self.assertIn('hostid = 46ba5037-7ce5-41fa-9452-48477bf00080', + content) + self.assertIn('hostsymname = lab-host-01', content) + self.assertIn('[Discovery Controller]', content) + self.assertIn( + 'controller = transport=tcp;traddr=192.168.1.10;trsvcid=8009', + content) + self.assertIn('[Subsystem]', content) + self.assertIn('nqn = nqn.2024-01.com.example:data.vol1', content) + self.assertIn('nr-io-queues = 4', content) + self.assertIn( + 'controller = transport=tcp;traddr=192.168.1.20;trsvcid=4420', + content) + # The well-known discovery NQN is omitted, not written literally. + self.assertNotIn('nqn = nqn.2014-08.org.nvmexpress.discovery', + content) + + def test_convert_legacy_bare_array_format(self): + self._write_json([{ + 'hostnqn': 'nqn.2014-08.org.nvmexpress:uuid:2222', + 'subsystems': [{ + 'nqn': 'nqn.2024-01.com.example:data.vol2', + 'ports': [{ + 'transport': 'tcp', + 'traddr': '10.0.0.5', + }], + }], + }]) + self._convert() + content = self._read_output() + self.assertIn('hostnqn = nqn.2014-08.org.nvmexpress:uuid:2222', + content) + self.assertIn('nqn = nqn.2024-01.com.example:data.vol2', content) + + def test_dhchap_default_inherited_by_port(self): + self._write_json({ + 'hosts': [{ + 'hostnqn': 'nqn.2014-08.org.nvmexpress:uuid:3333', + 'dhchap_key': 'DHHC-1:00:host-default-key:', + 'subsystems': [{ + 'nqn': 'nqn.2024-01.com.example:data.vol3', + 'ports': [{ + 'transport': 'tcp', + 'traddr': '10.0.0.6', + }], + }], + }], + }) + self._convert() + content = self._read_output() + self.assertIn('dhchap-secret = DHHC-1:00:host-default-key:', content) + + def test_dhchap_port_value_overrides_default(self): + self._write_json({ + 'hosts': [{ + 'hostnqn': 'nqn.2014-08.org.nvmexpress:uuid:4444', + 'dhchap_key': 'DHHC-1:00:host-default-key:', + 'subsystems': [{ + 'nqn': 'nqn.2024-01.com.example:data.vol4', + 'ports': [{ + 'transport': 'tcp', + 'traddr': '10.0.0.7', + 'dhchap_key': 'DHHC-1:00:port-specific-key:', + }], + }], + }], + }) + self._convert() + content = self._read_output() + self.assertIn('dhchap-secret = DHHC-1:00:port-specific-key:', content) + self.assertNotIn('DHHC-1:00:host-default-key:', content) + + # ------------------------------------------------------------------ # + # errors # + # ------------------------------------------------------------------ # + + def test_missing_explicit_config_file_errors(self): + missing = os.path.join(self.tmpdir, 'does-not-exist.json') + self._run('config-convert', '--config', missing, + '--output', self.output_ini, expect_fail=True) + self.assertFalse(os.path.exists(self.output_ini)) + + def test_malformed_json_errors(self): + with open(self.config_json, 'w') as f: + f.write('{not valid json') + self._convert(expect_fail=True) + self.assertFalse(os.path.exists(self.output_ini)) + + def test_missing_hosts_array_errors(self): + self._write_json({'not-hosts': []}) + self._convert(expect_fail=True) + self.assertFalse(os.path.exists(self.output_ini)) + + # ------------------------------------------------------------------ # + # output / force / rename # + # ------------------------------------------------------------------ # + + def test_refuses_to_overwrite_existing_output_without_force(self): + self._write_json({'hosts': []}) + with open(self.output_ini, 'w') as f: + f.write('preexisting content\n') + self._convert(expect_fail=True) + with open(self.output_ini) as f: + self.assertEqual(f.read(), 'preexisting content\n') + + def test_force_overwrites_existing_output(self): + self._write_json({ + 'hosts': [{ + 'subsystems': [{ + 'nqn': 'nqn.2024-01.com.example:data.vol5', + 'ports': [{'transport': 'tcp', 'traddr': '10.0.0.8'}], + }], + }], + }) + with open(self.output_ini, 'w') as f: + f.write('preexisting content\n') + self._convert('--force') + content = self._read_output() + self.assertIn('nqn = nqn.2024-01.com.example:data.vol5', content) + + def test_source_file_renamed_to_converted_on_success(self): + self._write_json({'hosts': []}) + self._convert() + self.assertFalse(os.path.exists(self.config_json)) + self.assertTrue(os.path.exists(self.config_json + '.converted')) + + def test_rerun_after_conversion_is_idempotent(self): + # A second run with the same --config, after config.json was + # already renamed to config.json.converted, must not choke on + # the source file being gone -- it was already converted, not + # missing. --force sidesteps the (unrelated) "output already + # exists" refusal so this test isolates just that behavior. + self._write_json({'hosts': []}) + self._convert() + result = self._convert('--force') + self.assertNotIn('failed to parse', result.stderr) + + +if __name__ == '__main__': + # Remove the binary path from argv so unittest.main() doesn't see it. + if len(sys.argv) >= 2 and not sys.argv[1].startswith('-'): + del sys.argv[1] + unittest.main()