libnvme: extract fabrics-specific logic into tree-fabrics.c#3365
Conversation
|
@igaw - Note: One class of checkpatch warning could not be resolved without introducing a different warning. The test file uses |
|
I like it :) We should check if the kdoc is actually showing up in the API doc, if not than this is good to go, otherwise I would say it's good time to address it when the code is moved around. I guess it is then '/* */' style doc. |
Move transport-specific ctrl matching code out of tree.c: - TCP matching functions and _candidate_init_fabrics() move to tree-fabrics.c (compiled only when CONFIG_FABRICS is set) - _match_ctrl() renamed to _tree_ctrl_match() (non-static) so it can be referenced from tree-fabrics.c - no-fabrics.c gets a _candidate_init_fabrics() stub returning NULL - streq0()/streqcase0() helpers move to private.h where both tree.c and tree-fabrics.c can reach them without an extra include - struct candidate_args / ctrl_match_t / related declarations move to private-fabrics.h; the iface_list field stays conditionally compiled on HAVE_NETDB||CONFIG_FABRICS while the rest of the struct is unconditional (portability fix for musl/HAVE_NETDB=0) Tests: - test/tree.c renamed to test/tree-fabrics.c (fabrics-only); move its meson registration under if want_fabrics inside the HAVE_NETDB block - test/tree-fabrics.c extended with 7 new test functions covering PCIe/loop matching, well-known NQN, "none" normalization, RDMA/FC config matching, and pagination - new test/tree.c added with 7 non-fabrics tests for host/subsystem creation, deduplication, iteration, and attribute getters; always compiled regardless of fabrics config Signed-off-by: Martin Belanger <Martin.Belanger@dell.com> Assisted-by: Claude:claude-sonnet-4-6 [Claude Code]
libnvme_read_sysfs_dhchap(), libnvme_read_sysfs_tls(), and libnvme_read_sysfs_tls_mode() are fabrics-only — the sysfs attributes they read (dhchap_secret, tls_key, tls_mode, …) are never present on PCIe controllers. Move them to tree-fabrics.c as static helpers under a single non-static wrapper libnvmf_read_sysfs_fabrics_attrs(), and rename all four with the libnvmf_ prefix to reflect their scope. Declare the wrapper in private-fabrics.h; add a no-op stub in no-fabrics.c. libnvme_reconfigure_ctrl() in tree.c now calls the wrapper instead of the three individual functions. Signed-off-by: Martin Belanger <Martin.Belanger@dell.com> Assisted-by: Claude:claude-sonnet-4-6 [Claude Code]
libnvme_ctrl_find() and _libnvme_ctrl_match_config() are only called from fabrics.c and are therefore fabrics-only. Rename them to libnvmf_ctrl_find() and _libnvmf_ctrl_match_config() to make that scope explicit. libnvme_ctrl_match_config() was the public wrapper around _libnvme_ctrl_match_config(). It was exported in libnvme.ld but has no callers outside of the library itself. Drop it entirely rather than carry it as dead public API. The internal _libnvmf_ctrl_match_config() is sufficient. Move the two remaining internal declarations from private.h to private-fabrics.h, where they belong alongside the other fabrics-internal helpers. Signed-off-by: Martin Belanger <Martin.Belanger@dell.com> Assisted-by: Claude:claude-sonnet-4-6 [Claude Code]
martin-belanger
force-pushed
the
extract-tree-fabrics
branch
from
146b049 to
f9fb597
Compare
The kdoc in |
|
Great cleanup. I like it! Thanks! |
2 participants
Summary
tree.ccurrently mixes PCIe/generic tree management with NVMe-oF-specific logic, including TCP address matching, TLS/DHChap sysfs readers, and controller lookup helpers that are only meaningful in a fabrics context. This series begins extracting that fabrics-specific code into a newtree-fabrics.c, compiled only whenwant_fabrics=true, following the existing*-fabrics.cconvention already established byutil-fabrics.candaccessors-fabrics.c.This is a preparation PR. Phases 1–3 are included here. Phase 4 (moving the full ctrl-find cluster:
libnvme_lookup_ctrl,libnvme_candidate_init,libnvme_ctrl_find,libnvme_tree_ctrl_match) will follow in a separate series once thelibnvmf_contextcoupling inlibnvme_ctrl_alloc()has been resolved.What this series does
Phase 1 — Create
tree-fabrics.c, eliminate#ifdef CONFIG_FABRICSfromtree.csrc/nvme/tree-fabrics.cand wires it intosrc/meson.build._tcp_*static matching functions out oftree.c._candidate_init()into a newlibnvmf_candidate_init()hook called from the generic path.libnvmf_candidate_init()stub tono-fabrics.c(returnsNULL).struct candidate_argsand thectrl_match_ttypedef toprivate-fabrics.h.tree.chas zero#ifdef CONFIG_FABRICSguards.Phase 2 — Move TLS/DHChap sysfs readers
libnvmf_read_sysfs_dhchap(),libnvmf_read_sysfs_tls(), andlibnvmf_read_sysfs_tls_mode()fromtree.ctotree-fabrics.casstatichelpers, behind a single non-static wrapperlibnvmf_read_sysfs_fabrics_attrs().libnvme_reconfigure_ctrl()intree.cis updated to call the wrapper.no-fabrics.c.Phase 3 — Move
libnvmf_ctrl_match_configandlibnvmf_ctrl_find; rename cleanuplibnvmf_ctrl_match_config()(previously_libnvme_ctrl_match_config) andlibnvmf_ctrl_find()(previously the 2-paramlibnvme_ctrl_find) are moved totree-fabrics.c. Both are purely fabrics-facing (all callers are infabrics.c).libnvme_ctrl_match_config()is dropped from the public API entirely. It was exported inlibnvme.ldbut had no external callers — nvme-cli does not call it, and nvme-stas goes through the Python bindings. It was dead API.libnvme_/libnvmf_prefixes:__nvme_ctrl_find→libnvme_ctrl_find(3-param),_candidate_init→libnvme_candidate_init,_tree_ctrl_match→libnvme_tree_ctrl_match,_libnvme_create_ctrl→libnvme_create_ctrl._discovery_config_json()infabrics.cis madestatic(it was accidentally non-static despite being file-local).no-fabrics.c— the moved functions have no callers outside the fabrics layer.What remains for the follow-up series (Phase 4)
The following functions still live in
tree.cbecauselibnvme_lookup_ctrl()is called by bothfabrics.candlibnvme_ctrl_alloc()(which constructs alibnvmf_contextas a parameter bag for all transport types, including PCIe). This coupling preventslibnvme_lookup_ctrl()and its dependencies from moving cleanly:libnvme_lookup_ctrl()libnvme_candidate_init()libnvme_ctrl_find()(3-param)libnvme_tree_ctrl_match()Phase 4 will introduce a PCIe-specific path in
libnvme_ctrl_alloc()that does not go throughlibnvmf_context, after which the full ctrl-find cluster can move totree-fabrics.candtree.cwill no longer need to includeprivate-fabrics.h.