From 88a5cfeb40ac7b59eacb853851fbd6f99c644a84 Mon Sep 17 00:00:00 2001 From: Daniel Wagner Date: Fri, 9 Jan 2026 08:46:52 +0100 Subject: [PATCH 1/6] fabrics: move config modify code to library Move configuration update code into the library. This completes the migration. Afterward, nvme-cli depends only on higher-level APIs, enabling internal library refactoring without breaking public interfaces. Signed-off-by: Daniel Wagner --- fabrics.c | 91 ++++++------------------------- libnvme/src/libnvme.ld | 1 + libnvme/src/nvme/fabrics.c | 107 +++++++++++++++++++++++++++---------- libnvme/src/nvme/fabrics.h | 13 +++++ 4 files changed, 109 insertions(+), 103 deletions(-) diff --git a/fabrics.c b/fabrics.c index 0720ebe841..9a1f0a0e98 100644 --- a/fabrics.c +++ b/fabrics.c @@ -584,34 +584,6 @@ int fabrics_discovery(const char *desc, int argc, char **argv, bool connect) return ret; } -static void nvme_parse_tls_args(const char *keyring, const char *tls_key, - const char *tls_key_identity, - struct nvme_fabrics_config *cfg, nvme_ctrl_t c) -{ - if (keyring) { - char *endptr; - long id = strtol(keyring, &endptr, 0); - - if (endptr != keyring) - cfg->keyring = id; - else - nvme_ctrl_set_keyring(c, keyring); - } - - if (tls_key_identity) - nvme_ctrl_set_tls_key_identity(c, tls_key_identity); - - if (tls_key) { - char *endptr; - long id = strtol(tls_key, &endptr, 0); - - if (endptr != tls_key) - cfg->tls_key = id; - else - nvme_ctrl_set_tls_key(c, tls_key); - } -} - int fabrics_connect(const char *desc, int argc, char **argv) { _cleanup_free_ char *hnqn = NULL; @@ -924,24 +896,15 @@ int fabrics_disconnect_all(const char *desc, int argc, char **argv) int fabrics_config(const char *desc, int argc, char **argv) { - char *subsysnqn = NULL; - char *transport = NULL, *traddr = NULL; - char *trsvcid = NULL, *hostnqn = NULL, *hostid = NULL; - char *host_traddr = NULL, *host_iface = NULL; - _cleanup_free_ char *hnqn = NULL; - _cleanup_free_ char *hid = NULL; - char *hostkey = NULL, *ctrlkey = NULL; - char *keyring = NULL, *tls_key = NULL, *tls_key_identity = NULL; - char *config_file = PATH_NVMF_CONFIG; - unsigned int verbose = 0; + bool scan_tree = false, modify_config = false, update_config = false; _cleanup_nvme_global_ctx_ struct nvme_global_ctx *ctx = NULL; - int ret; + char *config_file = PATH_NVMF_CONFIG; struct nvme_fabrics_config cfg; struct fabric_args fa = { }; - bool scan_tree = false, modify_config = false, update_config = false; + unsigned int verbose = 0; + int ret; NVMF_ARGS(opts, fa, cfg, - OPT_STRING("dhchap-ctrl-secret", 'C', "STR", &ctrlkey, nvmf_ctrlkey), OPT_STRING("config", 'J', "FILE", &config_file, nvmf_config_file), OPT_INCR("verbose", 'v', &verbose, "Increase logging verbosity"), OPT_FLAG("scan", 'R', &scan_tree, "Scan current NVMeoF topology"), @@ -980,52 +943,30 @@ int fabrics_config(const char *desc, int argc, char **argv) } if (modify_config) { - nvme_host_t h; - nvme_subsystem_t s; - nvme_ctrl_t c; + _cleanup_free_ struct nvmf_context *fctx = NULL; - if (!subsysnqn) { + if (!fa.subsysnqn) { fprintf(stderr, "required argument [--nqn | -n] needed with --modify\n"); return -EINVAL; } - if (!transport) { + if (!fa.transport) { fprintf(stderr, "required argument [--transport | -t] needed with --modify\n"); return -EINVAL; } - if (!hostnqn) - hostnqn = hnqn = nvmf_hostnqn_from_file(); - if (!hostid && hnqn) - hostid = hid = nvmf_hostid_from_file(); - h = nvme_lookup_host(ctx, hostnqn, hostid); - if (!h) { - fprintf(stderr, "Failed to lookup host '%s'\n", - hostnqn); - return -ENODEV; - } - if (hostkey) - nvme_host_set_dhchap_key(h, hostkey); - s = nvme_lookup_subsystem(h, NULL, subsysnqn); - if (!s) { - fprintf(stderr, "Failed to lookup subsystem '%s'\n", - subsysnqn); - return -ENODEV; - } - c = nvme_lookup_ctrl(s, transport, traddr, - host_traddr, host_iface, - trsvcid, NULL); - if (!c) { - fprintf(stderr, "Failed to lookup controller\n"); - return -ENODEV; - } - if (ctrlkey) - nvme_ctrl_set_dhchap_key(c, ctrlkey); - nvme_parse_tls_args(keyring, tls_key, tls_key_identity, &cfg, c); + ret = create_common_context(ctx, persistent, &fa, + &cfg, NULL, &fctx); + if (ret) + return ret; - nvmf_update_config(c, &cfg); + ret = nvmf_config_modify(ctx, fctx); + if (ret) { + fprintf(stderr, "failed to update config\n"); + return ret; + } } if (update_config) diff --git a/libnvme/src/libnvme.ld b/libnvme/src/libnvme.ld index 6a0be1e27e..2c0e47dd48 100644 --- a/libnvme/src/libnvme.ld +++ b/libnvme/src/libnvme.ld @@ -283,6 +283,7 @@ LIBNVME_2_0 { nvmf_add_ctrl; nvmf_adrfam_str; nvmf_cms_str; + nvmf_config_modify; nvmf_connect; nvmf_connect_config_json; nvmf_connect_ctrl; diff --git a/libnvme/src/nvme/fabrics.c b/libnvme/src/nvme/fabrics.c index e480976d33..8d5c6e9751 100644 --- a/libnvme/src/nvme/fabrics.c +++ b/libnvme/src/nvme/fabrics.c @@ -2115,6 +2115,35 @@ static int set_discovery_kato(struct nvmf_context *fctx, return tmo; } +static void nvme_parse_tls_args(const char *keyring, const char *tls_key, + const char *tls_key_identity, + struct nvme_fabrics_config *cfg, nvme_ctrl_t c) +{ + if (keyring) { + char *endptr; + long id = strtol(keyring, &endptr, 0); + + if (endptr != keyring) + cfg->keyring = id; + else + nvme_ctrl_set_keyring(c, keyring); + } + + if (tls_key_identity) + nvme_ctrl_set_tls_key_identity(c, tls_key_identity); + + if (tls_key) { + char *endptr; + long id = strtol(tls_key, &endptr, 0); + + if (endptr != tls_key) + cfg->tls_key = id; + else + nvme_ctrl_set_tls_key(c, tls_key); + } +} + + static int _nvmf_discovery(struct nvme_global_ctx *ctx, struct nvmf_context *fctx, bool connect, struct nvme_ctrl *c) @@ -2621,6 +2650,56 @@ int nvmf_discovery_config_file(struct nvme_global_ctx *ctx, return 0; } +int nvmf_config_modify(struct nvme_global_ctx *ctx, + struct nvmf_context *fctx) +{ + _cleanup_free_ char *hnqn = NULL; + _cleanup_free_ char *hid = NULL; + struct nvme_host *h; + struct nvme_subsystem *s; + struct nvme_ctrl *c; + + if (!fctx->hostnqn) + fctx->hostnqn = hnqn = nvmf_hostnqn_from_file(); + if (!fctx->hostid && hnqn) + fctx->hostid = hid = nvmf_hostid_from_file(); + + h = nvme_lookup_host(ctx, fctx->hostnqn, fctx->hostid); + if (!h) { + nvme_msg(ctx, LOG_ERR, "Failed to lookup host '%s'\n", + fctx->hostnqn); + return -ENODEV; + } + + if (fctx->hostkey) + nvme_host_set_dhchap_key(h, fctx->hostkey); + + s = nvme_lookup_subsystem(h, NULL, fctx->subsysnqn); + if (!s) { + nvme_msg(ctx, LOG_ERR, "Failed to lookup subsystem '%s'\n", + fctx->subsysnqn); + return -ENODEV; + } + + c = nvme_lookup_ctrl(s, fctx->transport, fctx->traddr, + fctx->host_traddr, fctx->host_iface, + fctx->trsvcid, NULL); + if (!c) { + nvme_msg(ctx, LOG_ERR, "Failed to lookup controller\n"); + return -ENODEV; + } + + if (fctx->ctrlkey) + nvme_ctrl_set_dhchap_key(c, fctx->ctrlkey); + + nvme_parse_tls_args(fctx->keyring, fctx->tls_key, + fctx->tls_key_identity, fctx->cfg, c); + + nvmf_update_config(c, fctx->cfg); + + return 0; +} + #define NBFT_SYSFS_FILENAME "NBFT*" static int nbft_filter(const struct dirent *dent) @@ -3283,34 +3362,6 @@ int nvmf_discovery(struct nvme_global_ctx *ctx, struct nvmf_context *fctx, return ret; } -static void nvme_parse_tls_args(const char *keyring, const char *tls_key, - const char *tls_key_identity, - struct nvme_fabrics_config *cfg, nvme_ctrl_t c) -{ - if (keyring) { - char *endptr; - long id = strtol(keyring, &endptr, 0); - - if (endptr != keyring) - cfg->keyring = id; - else - nvme_ctrl_set_keyring(c, keyring); - } - - if (tls_key_identity) - nvme_ctrl_set_tls_key_identity(c, tls_key_identity); - - if (tls_key) { - char *endptr; - long id = strtol(tls_key, &endptr, 0); - - if (endptr != tls_key) - cfg->tls_key = id; - else - nvme_ctrl_set_tls_key(c, tls_key); - } -} - int nvmf_connect(struct nvme_global_ctx *ctx, struct nvmf_context *fctx) { struct nvme_host *h; diff --git a/libnvme/src/nvme/fabrics.h b/libnvme/src/nvme/fabrics.h index c4d0ba7fba..c683ea3c56 100644 --- a/libnvme/src/nvme/fabrics.h +++ b/libnvme/src/nvme/fabrics.h @@ -632,6 +632,19 @@ int nvmf_connect(struct nvme_global_ctx *ctx, struct nvmf_context *fctx); int nvmf_connect_config_json(struct nvme_global_ctx *ctx, struct nvmf_context *fctx); +/** + * nvmf_config_modify() - Modify and update the configurtion + * @ctx: Global context + * @fctx: Fabrics context + * + * Update the current configuration by adding the crypto + * information. + * + * Return: 0 on success, or a negative error code on failure. + */ +int nvmf_config_modify(struct nvme_global_ctx *ctx, + struct nvmf_context *fctx); + /** * struct nbft_file_entry - Linked list entry for NBFT files * @next: Pointer to next entry From a5ce43da4443fd2ee4c26fc4deccef245784cbf4 Mon Sep 17 00:00:00 2001 From: Daniel Wagner Date: Fri, 9 Jan 2026 09:24:51 +0100 Subject: [PATCH 2/6] tree: remove unused default_host variable There is no user for this, thus remove it. Signed-off-by: Daniel Wagner --- libnvme/src/nvme/tree.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/libnvme/src/nvme/tree.c b/libnvme/src/nvme/tree.c index 67835f1ffb..8864874c6b 100644 --- a/libnvme/src/nvme/tree.c +++ b/libnvme/src/nvme/tree.c @@ -61,8 +61,6 @@ struct candidate_args { }; typedef bool (*ctrl_match_t)(struct nvme_ctrl *c, struct candidate_args *candidate); -static struct nvme_host *default_host; - static void __nvme_free_host(nvme_host_t h); static void __nvme_free_ctrl(nvme_ctrl_t c); static int nvme_subsystem_scan_namespace(struct nvme_global_ctx *ctx, @@ -211,7 +209,6 @@ int nvme_default_host(struct nvme_global_ctx *ctx, nvme_host_t *hp) nvme_host_set_hostsymname(h, NULL); - default_host = h; *hp = h; return 0; } From bc7286df5417010eae99e795a4d74ad0f42c48a0 Mon Sep 17 00:00:00 2001 From: Daniel Wagner Date: Fri, 9 Jan 2026 09:46:32 +0100 Subject: [PATCH 3/6] tree: add nvme_host_get Replace nvme_default_host() with generic nvme_host_get(), which creates the host object on demand. This enables retiring nvme_lookup_host() from the public API. Signed-off-by: Daniel Wagner --- libnvme/examples/discover-loop.c | 2 +- libnvme/libnvme/nvme.i | 4 +--- libnvme/src/libnvme.ld | 3 +-- libnvme/src/nvme/private.h | 3 +++ libnvme/src/nvme/tree.c | 37 +++++++++++++++----------------- libnvme/src/nvme/tree.h | 27 +++++++---------------- libnvme/test/tree.c | 30 +++++++++++++++----------- 7 files changed, 49 insertions(+), 57 deletions(-) diff --git a/libnvme/examples/discover-loop.c b/libnvme/examples/discover-loop.c index c3aa7c0cb1..367507d9d2 100644 --- a/libnvme/examples/discover-loop.c +++ b/libnvme/examples/discover-loop.c @@ -62,7 +62,7 @@ int main() ret = nvme_scan(NULL, &ctx); if (ret) return ret; - ret = nvme_default_host(ctx, &h); + ret = nvme_host_get(ctx, NULL, NULL, &h); if (ret) { fprintf(stderr, "Failed to allocated memory\n"); return ret; diff --git a/libnvme/libnvme/nvme.i b/libnvme/libnvme/nvme.i index a6fc37a76b..d877c2b6de 100644 --- a/libnvme/libnvme/nvme.i +++ b/libnvme/libnvme/nvme.i @@ -556,9 +556,7 @@ struct nvme_ns { const char *hostkey = NULL, const char *hostsymname = NULL) { nvme_host_t h; - if (hostnqn) - h = nvme_lookup_host(ctx, hostnqn, hostid); - if (nvme_default_host(ctx, &h)) + if (nvme_host_get(ctx, hostnqn, hostid, &h)) return NULL; if (hostsymname) nvme_host_set_hostsymname(h, hostsymname); diff --git a/libnvme/src/libnvme.ld b/libnvme/src/libnvme.ld index 2c0e47dd48..9b1c721ab8 100644 --- a/libnvme/src/libnvme.ld +++ b/libnvme/src/libnvme.ld @@ -56,7 +56,6 @@ LIBNVME_2_0 { nvme_ctrl_set_tls_key_identity; nvme_ctrl_set_unique_discovery_ctrl; nvme_ctrls_filter; - nvme_default_host; nvme_describe_key_serial; nvme_disconnect_ctrl; nvme_dump_config; @@ -100,6 +99,7 @@ LIBNVME_2_0 { nvme_get_telemetry_max; nvme_get_uuid_list; nvme_get_version; + nvme_host_get; nvme_host_get_dhchap_key; nvme_host_get_global_ctx; nvme_host_get_hostid; @@ -127,7 +127,6 @@ LIBNVME_2_0 { nvme_insert_tls_key_versioned; nvme_ipaddrs_eq; nvme_lookup_ctrl; - nvme_lookup_host; nvme_lookup_key; nvme_lookup_keyring; nvme_lookup_subsystem; diff --git a/libnvme/src/nvme/private.h b/libnvme/src/nvme/private.h index db7cebbe9f..e15fea0a14 100644 --- a/libnvme/src/nvme/private.h +++ b/libnvme/src/nvme/private.h @@ -411,6 +411,9 @@ void *__nvme_alloc(size_t len); void *__nvme_realloc(void *p, size_t len); +nvme_host_t nvme_lookup_host(struct nvme_global_ctx *ctx, const char *hostnqn, + const char *hostid); + #if (LOG_FUNCNAME == 1) #define __nvme_log_func __func__ #else diff --git a/libnvme/src/nvme/tree.c b/libnvme/src/nvme/tree.c index 8864874c6b..cb8cf0eab3 100644 --- a/libnvme/src/nvme/tree.c +++ b/libnvme/src/nvme/tree.c @@ -194,18 +194,19 @@ int nvme_host_get_ids(struct nvme_global_ctx *ctx, return 0; } -int nvme_default_host(struct nvme_global_ctx *ctx, nvme_host_t *hp) +int nvme_host_get(struct nvme_global_ctx *ctx, const char *hostnqn, + const char *hostid, nvme_host_t *hp) { - _cleanup_free_ char *hostnqn = NULL; - _cleanup_free_ char *hostid = NULL; + _cleanup_free_ char *hnqn = NULL; + _cleanup_free_ char *hid = NULL; struct nvme_host *h; int err; - err = nvme_host_get_ids(ctx, NULL, NULL, &hostnqn, &hostid); + err = nvme_host_get_ids(ctx, hostnqn, hostid, &hnqn, &hid); if (err) return err; - h = nvme_lookup_host(ctx, hostnqn, hostid); + h = nvme_lookup_host(ctx, hnqn, hid); nvme_host_set_hostsymname(h, NULL); @@ -888,7 +889,7 @@ static int nvme_scan_subsystem(struct nvme_global_ctx *ctx, const char *name) */ nvme_msg(ctx, LOG_DEBUG, "creating detached subsystem '%s'\n", name); - ret = nvme_default_host(ctx, &h); + ret = nvme_host_get(ctx, NULL, NULL, &h); if (ret) return ret; s = nvme_alloc_subsystem(h, name, subsysnqn); @@ -2255,21 +2256,17 @@ int nvme_scan_ctrl(struct nvme_global_ctx *ctx, const char *name, hostnqn = nvme_get_attr(path, "hostnqn"); hostid = nvme_get_attr(path, "hostid"); - h = nvme_lookup_host(ctx, hostnqn, hostid); - if (h) { - host_key = nvme_get_attr(path, "dhchap_secret"); - if (host_key && strcmp(host_key, "none")) { - free(h->dhchap_key); - h->dhchap_key = host_key; - host_key = NULL; - } - free(host_key); - } - if (!h) { - ret = nvme_default_host(ctx, &h); - if (ret) - return ret; + ret = nvme_host_get(ctx, hostnqn, hostid, &h); + if (ret) + return ret; + + host_key = nvme_get_attr(path, "dhchap_secret"); + if (host_key && strcmp(host_key, "none")) { + free(h->dhchap_key); + h->dhchap_key = host_key; + host_key = NULL; } + free(host_key); subsysnqn = nvme_get_attr(path, "subsysnqn"); if (!subsysnqn) diff --git a/libnvme/src/nvme/tree.h b/libnvme/src/nvme/tree.h index 469dc29495..3ae7ae2635 100644 --- a/libnvme/src/nvme/tree.h +++ b/libnvme/src/nvme/tree.h @@ -96,20 +96,6 @@ nvme_host_t nvme_next_host(struct nvme_global_ctx *ctx, nvme_host_t h); */ struct nvme_global_ctx *nvme_host_get_global_ctx(nvme_host_t h); -/** - * nvme_lookup_host() - Lookup nvme_host_t object - * @ctx: struct nvme_global_ctx object - * @hostnqn: Host NQN - * @hostid: Host ID - * - * Lookup a nvme_host_t object based on @hostnqn and @hostid - * or create one if not found. - * - * Return: &nvme_host_t object - */ -nvme_host_t nvme_lookup_host(struct nvme_global_ctx *ctx, const char *hostnqn, - const char *hostid); - /** * nvme_host_get_dhchap_key() - Return host key * @h: Host for which the key should be returned @@ -148,16 +134,19 @@ void nvme_host_set_pdc_enabled(nvme_host_t h, bool enabled); bool nvme_host_is_pdc_enabled(nvme_host_t h, bool fallback); /** - * nvme_default_host() - Initializes the default host + * nvme_host_get() - Returns a host object * @ctx: struct nvme_global_ctx object - * @h: &nvme_host_t object to return + * @hostnqn: Host NQN (optional) + * @hostid: Host ID (optional) + * @h: &nvme_host_t object to return * - * Initializes the default host object based on the hostnqn/hostid - * values returned by nvme_host_get_ids() and attaches it to @r. + * Returns a host object based on the hostnqn/hostid values or the default if + * hostnqn/hostid are NULL. * * Return: 0 on success or negative error code otherwise */ -int nvme_default_host(struct nvme_global_ctx *ctx, nvme_host_t *h); +int nvme_host_get(struct nvme_global_ctx *ctx, const char *hostnqn, + const char *hostid, nvme_host_t *h); /** * nvme_host_get_ids - Retrieve host ids from various sources diff --git a/libnvme/test/tree.c b/libnvme/test/tree.c index a8191e8124..dde4905581 100644 --- a/libnvme/test/tree.c +++ b/libnvme/test/tree.c @@ -17,6 +17,8 @@ struct test_data { /* input data */ + const char *hostnqn; + const char *hostid; const char *subsysname; const char *subsysnqn; const char *transport; @@ -31,23 +33,27 @@ struct test_data { int ctrl_id; }; +#define DEFAULT_HOSTID "9ba1651a-ed36-11f0-9858-6c1ff71ba506" +#define DEFAULT_HOSTNQN "nqn.2014-08.org.nvmexpress:uuid:DEFAULT_HOSTID" #define DEFAULT_SUBSYSNAME "subsysname" #define DEFAULT_SUBSYSNQN "subsysnqn" #define SRC_ADDR4 "192.168.56.100" #define SRC_ADDR6 "1234:5678:abcd:EF01:1234:5678:abcd:EF01" +#define DEFAULTS DEFAULT_HOSTNQN, DEFAULT_HOSTNQN, DEFAULT_SUBSYSNAME, DEFAULT_SUBSYSNQN + struct test_data test_data[] = { - { DEFAULT_SUBSYSNAME, DEFAULT_SUBSYSNQN, "tcp", "192.168.1.1", "192.168.1.20", NULL, "4420" }, - { DEFAULT_SUBSYSNAME, DEFAULT_SUBSYSNQN, "tcp", "192.168.1.1", "192.168.1.20", NULL, "4421" }, - { DEFAULT_SUBSYSNAME, DEFAULT_SUBSYSNQN, "tcp", "192.168.1.2", "192.168.1.20", "eth1", "4420" }, - { DEFAULT_SUBSYSNAME, DEFAULT_SUBSYSNQN, "tcp", "192.168.1.2", "192.168.1.20", "eth1", "4421" }, - { DEFAULT_SUBSYSNAME, DEFAULT_SUBSYSNQN, "rdma", "192.168.1.3", "192.168.1.20", NULL, NULL }, - { DEFAULT_SUBSYSNAME, DEFAULT_SUBSYSNQN, "rdma", "192.168.1.4", "192.168.1.20", NULL, NULL }, - { DEFAULT_SUBSYSNAME, DEFAULT_SUBSYSNQN, "fc", + { DEFAULTS, "tcp", "192.168.1.1", "192.168.1.20", NULL, "4420" }, + { DEFAULTS, "tcp", "192.168.1.1", "192.168.1.20", NULL, "4421" }, + { DEFAULTS, "tcp", "192.168.1.2", "192.168.1.20", "eth1", "4420" }, + { DEFAULTS, "tcp", "192.168.1.2", "192.168.1.20", "eth1", "4421" }, + { DEFAULTS, "rdma", "192.168.1.3", "192.168.1.20", NULL, NULL }, + { DEFAULTS, "rdma", "192.168.1.4", "192.168.1.20", NULL, NULL }, + { DEFAULTS, "fc", "nn-0x201700a09890f5bf:pn-0x201900a09890f5bf", "nn-0x200000109b579ef3:pn-0x100000109b579ef3" }, - { DEFAULT_SUBSYSNAME, DEFAULT_SUBSYSNQN, "fc", + { DEFAULTS, "fc", "nn-0x201700a09890f5bf:pn-0x201900a09890f5bf", "nn-0x200000109b579ef6:pn-0x100000109b579ef6", }, @@ -125,7 +131,7 @@ static struct nvme_global_ctx *create_tree() ctx = nvme_create_global_ctx(stdout, LOG_DEBUG); assert(ctx); - nvme_default_host(ctx, &h); + nvme_host_get(ctx, DEFAULT_HOSTNQN, DEFAULT_HOSTID, &h); assert(h); printf(" ctrls created:\n"); @@ -281,7 +287,7 @@ static bool test_src_addr() ctx = nvme_create_global_ctx(stdout, LOG_DEBUG); assert(ctx); - nvme_default_host(ctx, &h); + nvme_host_get(ctx, DEFAULT_HOSTNQN, DEFAULT_HOSTID, &h); assert(h); s = nvme_lookup_subsystem(h, DEFAULT_SUBSYSNAME, DEFAULT_SUBSYSNQN); @@ -457,7 +463,7 @@ static bool ctrl_match(const char *tag, ctx = nvme_create_global_ctx(stdout, LOG_INFO); assert(ctx); - nvme_default_host(ctx, &h); + nvme_host_get(ctx, DEFAULT_HOSTNQN, DEFAULT_HOSTID, &h); assert(h); s = nvme_lookup_subsystem(h, DEFAULT_SUBSYSNAME, reference->subsysnqn ? reference->subsysnqn : DEFAULT_SUBSYSNQN); @@ -1070,7 +1076,7 @@ static bool ctrl_config_match(const char *tag, ctx = nvme_create_global_ctx(stdout, LOG_INFO); assert(ctx); - nvme_default_host(ctx, &h); + nvme_host_get(ctx, DEFAULT_HOSTNQN, DEFAULT_HOSTID, &h); assert(h); s = nvme_lookup_subsystem(h, DEFAULT_SUBSYSNAME, reference->subsysnqn ? reference->subsysnqn : DEFAULT_SUBSYSNQN); From f01e4119f8b3c077ddd0fe4b410da0cb99a77317 Mon Sep 17 00:00:00 2001 From: Daniel Wagner Date: Fri, 9 Jan 2026 10:02:06 +0100 Subject: [PATCH 4/6] tree: separate create host code from nvme_lookup_host Split host creation code into a separate function to explicitly document on-demand object creation. Signed-off-by: Daniel Wagner --- libnvme/src/nvme/tree.c | 45 +++++++++++++++++++++++++++++------------ libnvme/test/tree.c | 2 +- 2 files changed, 33 insertions(+), 14 deletions(-) diff --git a/libnvme/src/nvme/tree.c b/libnvme/src/nvme/tree.c index cb8cf0eab3..a3c3159b5e 100644 --- a/libnvme/src/nvme/tree.c +++ b/libnvme/src/nvme/tree.c @@ -195,7 +195,7 @@ int nvme_host_get_ids(struct nvme_global_ctx *ctx, } int nvme_host_get(struct nvme_global_ctx *ctx, const char *hostnqn, - const char *hostid, nvme_host_t *hp) + const char *hostid, nvme_host_t *host) { _cleanup_free_ char *hnqn = NULL; _cleanup_free_ char *hid = NULL; @@ -207,10 +207,12 @@ int nvme_host_get(struct nvme_global_ctx *ctx, const char *hostnqn, return err; h = nvme_lookup_host(ctx, hnqn, hid); + if (!h) + return -ENOMEM; nvme_host_set_hostsymname(h, NULL); - *hp = h; + *host = h; return 0; } @@ -762,13 +764,37 @@ void nvme_free_host(struct nvme_host *h) __nvme_free_host(h); } -struct nvme_host *nvme_lookup_host(struct nvme_global_ctx *ctx, const char *hostnqn, - const char *hostid) +static int nvme_create_host(struct nvme_global_ctx *ctx, const char *hostnqn, + const char *hostid, struct nvme_host **host) +{ + struct nvme_host *h; + + h = calloc(1, sizeof(*h)); + if (!h) + return -ENOMEM; + + h->hostnqn = strdup(hostnqn); + if (hostid) + h->hostid = strdup(hostid); + list_head_init(&h->subsystems); + list_node_init(&h->entry); + h->ctx = ctx; + + list_add_tail(&ctx->hosts, &h->entry); + + *host = h; + + return 0; +} + +struct nvme_host *nvme_lookup_host(struct nvme_global_ctx *ctx, + const char *hostnqn, const char *hostid) { struct nvme_host *h; if (!hostnqn) return NULL; + nvme_for_each_host(ctx, h) { if (strcmp(h->hostnqn, hostnqn)) continue; @@ -777,16 +803,9 @@ struct nvme_host *nvme_lookup_host(struct nvme_global_ctx *ctx, const char *host continue; return h; } - h = calloc(1,sizeof(*h)); - if (!h) + + if (nvme_create_host(ctx, hostnqn, hostid, &h)) return NULL; - h->hostnqn = strdup(hostnqn); - if (hostid) - h->hostid = strdup(hostid); - list_head_init(&h->subsystems); - list_node_init(&h->entry); - h->ctx = ctx; - list_add_tail(&ctx->hosts, &h->entry); return h; } diff --git a/libnvme/test/tree.c b/libnvme/test/tree.c index dde4905581..f9f394d27d 100644 --- a/libnvme/test/tree.c +++ b/libnvme/test/tree.c @@ -34,7 +34,7 @@ struct test_data { }; #define DEFAULT_HOSTID "9ba1651a-ed36-11f0-9858-6c1ff71ba506" -#define DEFAULT_HOSTNQN "nqn.2014-08.org.nvmexpress:uuid:DEFAULT_HOSTID" +#define DEFAULT_HOSTNQN "nqn.2014-08.org.nvmexpress:uuid:9ba1651a-ed36-11f0-9858-6c1ff71ba506" #define DEFAULT_SUBSYSNAME "subsysname" #define DEFAULT_SUBSYSNQN "subsysnqn" #define SRC_ADDR4 "192.168.56.100" From 4755fb2e6d05aae459503696ef5f77c2a7316d98 Mon Sep 17 00:00:00 2001 From: Daniel Wagner Date: Fri, 9 Jan 2026 10:22:10 +0100 Subject: [PATCH 5/6] tree: add nvme_subsystem_get Replace the nvme_lookup_subsystem function with the nvme_subsystem_get function, which creates the subsystem object if necessary. This allows retiring the nvme_lookup_subsystem function from the public API. Signed-off-by: Daniel Wagner --- libnvme/libnvme/nvme.i | 13 ++++++++++--- libnvme/src/libnvme.ld | 2 +- libnvme/src/nvme/private.h | 4 ++++ libnvme/src/nvme/tree.c | 15 +++++++++++++++ libnvme/src/nvme/tree.h | 13 +++++++------ libnvme/test/tree.c | 17 ++++++++++++----- 6 files changed, 49 insertions(+), 15 deletions(-) diff --git a/libnvme/libnvme/nvme.i b/libnvme/libnvme/nvme.i index d877c2b6de..fc13bf679e 100644 --- a/libnvme/libnvme/nvme.i +++ b/libnvme/libnvme/nvme.i @@ -600,15 +600,22 @@ struct nvme_ns { } %}; -%pythonappend nvme_subsystem::nvme_subsystem(struct nvme_host *host, +%pythonappend nvme_subsystem::nvme_subsystem(struct nvme_global_ctx *ctx, + struct nvme_host *host, const char *subsysnqn, const char *name) { self.__parent = host # Keep a reference to parent to ensure garbage collection happens in the right order} %extend nvme_subsystem { - nvme_subsystem(struct nvme_host *host, + nvme_subsystem(struct nvme_global_ctx *ctx, + struct nvme_host *host, const char *subsysnqn, const char *name = NULL) { - return nvme_lookup_subsystem(host, name, subsysnqn); + struct nvme_subsystem *s; + + if (nvme_subsystem_get(ctx, host, name, subsysnqn, &s)) + return NULL; + + return s; } ~nvme_subsystem() { nvme_free_subsystem($self); diff --git a/libnvme/src/libnvme.ld b/libnvme/src/libnvme.ld index 9b1c721ab8..e3bb2c916c 100644 --- a/libnvme/src/libnvme.ld +++ b/libnvme/src/libnvme.ld @@ -129,7 +129,6 @@ LIBNVME_2_0 { nvme_lookup_ctrl; nvme_lookup_key; nvme_lookup_keyring; - nvme_lookup_subsystem; nvme_mi_admin_admin_passthru; nvme_mi_admin_xfer; nvme_mi_aem_disable; @@ -248,6 +247,7 @@ LIBNVME_2_0 { nvme_subsys_filter; nvme_subsystem_first_ctrl; nvme_subsystem_first_ns; + nvme_subsystem_get; nvme_subsystem_get_application; nvme_subsystem_get_fw_rev; nvme_subsystem_get_host; diff --git a/libnvme/src/nvme/private.h b/libnvme/src/nvme/private.h index e15fea0a14..310c4ede19 100644 --- a/libnvme/src/nvme/private.h +++ b/libnvme/src/nvme/private.h @@ -413,6 +413,10 @@ void *__nvme_realloc(void *p, size_t len); nvme_host_t nvme_lookup_host(struct nvme_global_ctx *ctx, const char *hostnqn, const char *hostid); +nvme_subsystem_t nvme_lookup_subsystem(struct nvme_host *h, + const char *name, + const char *subsysnqn); + #if (LOG_FUNCNAME == 1) #define __nvme_log_func __func__ diff --git a/libnvme/src/nvme/tree.c b/libnvme/src/nvme/tree.c index a3c3159b5e..60da9aff09 100644 --- a/libnvme/src/nvme/tree.c +++ b/libnvme/src/nvme/tree.c @@ -736,6 +736,21 @@ struct nvme_subsystem *nvme_lookup_subsystem(struct nvme_host *h, return nvme_alloc_subsystem(h, name, subsysnqn); } +int nvme_subsystem_get(struct nvme_global_ctx *ctx, + struct nvme_host *h, const char *name, + const char *subsysnqn, struct nvme_subsystem **subsys) +{ + struct nvme_subsystem *s; + + s = nvme_lookup_subsystem(h, name, subsysnqn); + if (!s) + return -ENOMEM; + + *subsys = s; + + return 0; +} + static void __nvme_free_host(struct nvme_host *h) { struct nvme_subsystem *s, *_s; diff --git a/libnvme/src/nvme/tree.h b/libnvme/src/nvme/tree.h index 3ae7ae2635..616deebece 100644 --- a/libnvme/src/nvme/tree.h +++ b/libnvme/src/nvme/tree.h @@ -201,19 +201,20 @@ nvme_subsystem_t nvme_first_subsystem(nvme_host_t h); nvme_subsystem_t nvme_next_subsystem(nvme_host_t h, nvme_subsystem_t s); /** - * nvme_lookup_subsystem() - Lookup nvme_subsystem_t object + * nvme_subsystem_get() - Returns nvme_subsystem_t object + * @ctx: struct nvme_global_ctx object * @h: &nvme_host_t object * @name: Name of the subsystem (may be NULL) * @subsysnqn: Subsystem NQN + * @s: nvme_subsystem_t object * - * Lookup a &nvme_subsystem_t object in @h base on @name (if present) + * Returns an &nvme_subsystem_t object in @h base on @name (if present) * and @subsysnqn or create one if not found. * - * Return: nvme_subsystem_t object */ -nvme_subsystem_t nvme_lookup_subsystem(struct nvme_host *h, - const char *name, - const char *subsysnqn); +int nvme_subsystem_get(struct nvme_global_ctx *ctx, + struct nvme_host *h, const char *name, + const char *subsysnqn, struct nvme_subsystem **s); /** * nvme_free_subsystem() - Free a subsystem diff --git a/libnvme/test/tree.c b/libnvme/test/tree.c index f9f394d27d..a0ad0a5ad7 100644 --- a/libnvme/test/tree.c +++ b/libnvme/test/tree.c @@ -138,7 +138,8 @@ static struct nvme_global_ctx *create_tree() for (int i = 0; i < ARRAY_SIZE(test_data); i++) { struct test_data *d = &test_data[i]; - d->s = nvme_lookup_subsystem(h, d->subsysname, d->subsysnqn); + assert(!nvme_subsystem_get(ctx, h, d->subsysname, + d->subsysnqn, &d->s)); assert(d->s); d->c = nvme_lookup_ctrl(d->s, d->transport, d->traddr, d->host_traddr, d->host_iface, @@ -235,7 +236,7 @@ static bool ctrl_lookups(struct nvme_global_ctx *ctx) bool pass = true; h = nvme_first_host(ctx); - s = nvme_lookup_subsystem(h, DEFAULT_SUBSYSNAME, DEFAULT_SUBSYSNQN); + nvme_subsystem_get(ctx, h, DEFAULT_SUBSYSNAME, DEFAULT_SUBSYSNQN, &s); printf(" lookup controller:\n"); for (int i = 0; i < ARRAY_SIZE(test_data); i++) { @@ -290,7 +291,7 @@ static bool test_src_addr() nvme_host_get(ctx, DEFAULT_HOSTNQN, DEFAULT_HOSTID, &h); assert(h); - s = nvme_lookup_subsystem(h, DEFAULT_SUBSYSNAME, DEFAULT_SUBSYSNQN); + nvme_subsystem_get(ctx, h, DEFAULT_SUBSYSNAME, DEFAULT_SUBSYSNQN, &s); assert(s); c = nvme_lookup_ctrl(s, "tcp", "192.168.56.1", NULL, NULL, "8009", NULL); @@ -466,7 +467,10 @@ static bool ctrl_match(const char *tag, nvme_host_get(ctx, DEFAULT_HOSTNQN, DEFAULT_HOSTID, &h); assert(h); - s = nvme_lookup_subsystem(h, DEFAULT_SUBSYSNAME, reference->subsysnqn ? reference->subsysnqn : DEFAULT_SUBSYSNQN); + assert(!nvme_subsystem_get(ctx, h, DEFAULT_SUBSYSNAME, + reference->subsysnqn ? + reference->subsysnqn : DEFAULT_SUBSYSNQN, + &s)); assert(s); reference_ctrl = nvme_lookup_ctrl(s, reference->transport, reference->traddr, @@ -1079,7 +1083,10 @@ static bool ctrl_config_match(const char *tag, nvme_host_get(ctx, DEFAULT_HOSTNQN, DEFAULT_HOSTID, &h); assert(h); - s = nvme_lookup_subsystem(h, DEFAULT_SUBSYSNAME, reference->subsysnqn ? reference->subsysnqn : DEFAULT_SUBSYSNQN); + assert(!nvme_subsystem_get(ctx, h, DEFAULT_SUBSYSNAME, + reference->subsysnqn ? + reference->subsysnqn : DEFAULT_SUBSYSNQN, + &s)); assert(s); reference_ctrl = nvme_lookup_ctrl(s, reference->transport, reference->traddr, From a35dfc48a173d460fdbb85d300a85a68875889dd Mon Sep 17 00:00:00 2001 From: Daniel Wagner Date: Fri, 9 Jan 2026 10:50:50 +0100 Subject: [PATCH 6/6] tree: make nvme_lookup_ctrl and nvme_ctrl_find private There are no public users for these function. Make them private so we can refactor them without breaking the API in future. Signed-off-by: Daniel Wagner --- libnvme/src/libnvme.ld | 2 -- libnvme/src/nvme/private.h | 9 ++++++- libnvme/src/nvme/tree.h | 48 -------------------------------------- libnvme/test/meson.build | 2 +- 4 files changed, 9 insertions(+), 52 deletions(-) diff --git a/libnvme/src/libnvme.ld b/libnvme/src/libnvme.ld index e3bb2c916c..f4d271afdd 100644 --- a/libnvme/src/libnvme.ld +++ b/libnvme/src/libnvme.ld @@ -9,7 +9,6 @@ LIBNVME_2_0 { nvme_create_ctrl; nvme_create_global_ctx; nvme_ctrl_config_match; - nvme_ctrl_find; nvme_ctrl_first_ns; nvme_ctrl_first_path; nvme_ctrl_get_address; @@ -126,7 +125,6 @@ LIBNVME_2_0 { nvme_insert_tls_key_compat; nvme_insert_tls_key_versioned; nvme_ipaddrs_eq; - nvme_lookup_ctrl; nvme_lookup_key; nvme_lookup_keyring; nvme_mi_admin_admin_passthru; diff --git a/libnvme/src/nvme/private.h b/libnvme/src/nvme/private.h index 310c4ede19..c27e9c3858 100644 --- a/libnvme/src/nvme/private.h +++ b/libnvme/src/nvme/private.h @@ -416,7 +416,14 @@ nvme_host_t nvme_lookup_host(struct nvme_global_ctx *ctx, const char *hostnqn, nvme_subsystem_t nvme_lookup_subsystem(struct nvme_host *h, const char *name, const char *subsysnqn); - +nvme_ctrl_t nvme_lookup_ctrl(nvme_subsystem_t s, const char *transport, + const char *traddr, const char *host_traddr, + const char *host_iface, const char *trsvcid, + nvme_ctrl_t p); +nvme_ctrl_t nvme_ctrl_find(nvme_subsystem_t s, const char *transport, + const char *traddr, const char *trsvcid, + const char *subsysnqn, const char *host_traddr, + const char *host_iface); #if (LOG_FUNCNAME == 1) #define __nvme_log_func __func__ diff --git a/libnvme/src/nvme/tree.h b/libnvme/src/nvme/tree.h index 616deebece..bf7420759c 100644 --- a/libnvme/src/nvme/tree.h +++ b/libnvme/src/nvme/tree.h @@ -300,54 +300,6 @@ nvme_path_t nvme_namespace_first_path(nvme_ns_t ns); */ nvme_path_t nvme_namespace_next_path(nvme_ns_t ns, nvme_path_t p); -/** - * nvme_lookup_ctrl() - Lookup nvme_ctrl_t object - * @s: &nvme_subsystem_t object - * @transport: Transport name - * @traddr: Transport address - * @host_traddr: Host transport address - * @host_iface: Host interface name - * @trsvcid: Transport service identifier - * @p: Previous controller instance - * - * Lookup a controller in @s based on @transport, @traddr, - * @host_traddr, @host_iface, and @trsvcid. @transport must be specified, - * other fields may be required depending on the transport. A new - * object is created if none is found. If @p is specified the lookup - * will start at @p instead of the first controller. - * - * Return: Controller instance - */ -nvme_ctrl_t nvme_lookup_ctrl(nvme_subsystem_t s, const char *transport, - const char *traddr, const char *host_traddr, - const char *host_iface, const char *trsvcid, - nvme_ctrl_t p); - -/** - * nvme_ctrl_find() - Locate an existing controller - * @s: &nvme_subsystem_t object - * @transport: Transport name - * @traddr: Transport address - * @trsvcid: Transport service identifier - * @subsysnqn: Subsystem NQN - * @host_traddr: Host transport address - * @host_iface: Host interface name - * - * Lookup a controller in @s based on @transport, @traddr, @trsvcid, - * @subsysnqn, @host_traddr, and @host_iface. @transport must be specified, - * other fields may be required depending on the transport. Parameters set - * to NULL will be ignored. - * - * Unlike nvme_lookup_ctrl(), this function does not create a new object if - * an existing controller cannot be found. - * - * Return: Controller instance on success, NULL otherwise. - */ -nvme_ctrl_t nvme_ctrl_find(nvme_subsystem_t s, const char *transport, - const char *traddr, const char *trsvcid, - const char *subsysnqn, const char *host_traddr, - const char *host_iface); - /** * nvme_ctrl_config_match() - Check if ctrl @c matches config params * @c: An existing controller instance diff --git a/libnvme/test/meson.build b/libnvme/test/meson.build index 6512f5b959..5b21e8f557 100644 --- a/libnvme/test/meson.build +++ b/libnvme/test/meson.build @@ -130,7 +130,7 @@ if conf.get('HAVE_NETDB') dependencies: [ config_dep, ccan_dep, - libnvme_dep, + libnvme_test_dep, ], link_with: mock_ifaddrs, )