Make struct libnvme_fabrics_config private#3273
Merged
igaw merged 17 commits intolinux-nvme:masterfrom Apr 16, 2026
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to remove struct libnvme_fabrics_config from the public/stable API surface by moving its definition into private headers, while introducing new fabrics-context-based flows and accessor generation updates to keep configuration usable without exposing struct layout.
Changes:
- Move
struct libnvme_fabrics_configout of the public fabrics header and update consumers to use fabrics-context/controller config getters. - Extend the accessor generator to support struct-level default modes (
both/none/readonly/writeonly) plus per-member overrides. - Update libnvme/libnvmf APIs, examples, tests, and Python bindings to create controllers via
libnvmf_contextand new helper APIs.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| libnvme/tools/generator/update-accessors.sh | Adjusts symbol drift output formatting for version scripts. |
| libnvme/tools/generator/generate-accessors.py | Adds struct-level accessor modes and per-member mode overrides. |
| libnvme/tools/generator/generate-accessors.md | Documents new accessor-generation modes and annotations. |
| libnvme/test/tree.c | Includes fabrics private header for tests. |
| libnvme/src/nvme/tree.h | Removes some previously public controller/config APIs from the public header. |
| libnvme/src/nvme/tree.c | Refactors controller creation/config initialization and moves fabrics helpers out. |
| libnvme/src/nvme/private.h | Moves struct libnvme_fabrics_config into private header and updates internal prototypes. |
| libnvme/src/nvme/private-fabrics.h | Introduces/moves struct libnvmf_context definition and fabrics-private helpers. |
| libnvme/src/nvme/no-fabrics.c | Adds non-fabrics stub for hostname detection. |
| libnvme/src/nvme/linux.c | Updates renamed config fields (e.g., keyring_id). |
| libnvme/src/nvme/json.c | Switches to fabrics-config getter API for JSON config handling. |
| libnvme/src/nvme/fabrics.h | Removes public config struct; adds new public getter APIs and controller lifecycle APIs. |
| libnvme/src/nvme/fabrics.c | Implements new config getter APIs, disconnect helper, and refactors config handling. |
| libnvme/src/nvme/accessors.h / accessors.c | Adds generated public accessors for struct libnvme_fabrics_config (opaque in headers). |
| libnvme/src/nvme/accessors-fabrics.h / accessors-fabrics.c | Formatting/wrapping changes in generated accessors. |
| libnvme/src/meson.build | Builds no-fabrics.c when fabrics are disabled. |
| libnvme/src/libnvmf.ld / libnvme/src/libnvme.ld / libnvme/src/accessors.ld | Updates exported symbol lists for the new/removed APIs. |
| libnvme/libnvme/tests/test-objects.py | Updates Python tests to create controllers via fabrics_context. |
| libnvme/libnvme/tests/gc.py / create-ctrl-obj.py | Updates Python scripts to use fabrics_context. |
| libnvme/libnvme/nvme.i | Adds Python fabrics_context wrapper and updates controller creation/connect/disconnect. |
| libnvme/examples/discover-loop.c | Updates example to use libnvmf_context + new create/add/disconnect APIs. |
| fabrics.c | Updates nvme-cli fabrics tooling to set fabrics options via accessors/context config. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
igaw
force-pushed
the
fabrics-options
branch
3 times, most recently
from
f741ff8 to
b55e144
Compare
Extend the !generate-accessors annotation so callers can set a
struct-level default for accessor generation:
struct foo { //!generate-accessors - both get+set (unchanged)
struct foo { //!generate-accessors:none - no accessors by default
struct foo { //!generate-accessors:readonly - getter only by default
struct foo { //!generate-accessors:writeonly - setter only by default
Add two new per-member annotations to complement the new struct-level
defaults when individual members need to override them:
//!accessors:writeonly - setter only for this member
//!accessors:readwrite - both getter and setter for this member
Internally, replace Member.is_const with explicit gen_getter/gen_setter
flags and add parse_struct_annotation() to extract the mode qualifier.
Update generate-accessors.md with the full annotation table.
Signed-off-by: Daniel Wagner <wagi@kernel.org>
The JSON code is used for reading and writing the configuration files used by the fabrics code. Therefore, it does not make sense to enable JSON without fabrics. This simplifies the core dependencies so the JSON- related code can safely assume fabrics is enabled. Signed-off-by: Daniel Wagner <wagi@kernel.org>
This function exists in two files with slightly different implementations. Use a single shared implementation instead. Signed-off-by: Daniel Wagner <wagi@kernel.org>
Prepare the stage for generating getter/setters for libnvmf_context. For this, the struct needs to be in the private-fabrics header so the getter/setters are placed in the correct header file. Signed-off-by: Daniel Wagner <wagi@kernel.org>
merge_config merges libnvme_fabrics_config into the controller object from the first argument. There is no need to return the config, and there is no user for it. Signed-off-by: Daniel Wagner <wagi@kernel.org>
Teach the Python binding about libnvmf_context so that libnvme_fabric_options can eventually be retired. Signed-off-by: Daniel Wagner <wagi@kernel.org>
libnvme_fabrics_config is going to be removed from the API. The first step is to let libnvmf_context own the configuration, coupling the fabrics config lifetime to the context object. Signed-off-by: Daniel Wagner <wagi@kernel.org>
Retire the libnvme_fabrics_options type from the public API as any modification is breaking ABI. The libnvmf_context has been introduced as replacement, with an extendable getter/setter API. Signed-off-by: Daniel Wagner <wagi@kernel.org>
The connect and disconnect functions belong to the fabrics API. Move them there. While at it, also move libnvme_ctrl_get_config to the fabrics.h header, since it returns the fabrics configuration. Signed-off-by: Daniel Wagner <wagi@kernel.org>
There are two types of keys used: one is the string representation and the other is the actual key ID used by the kernel. Add a postfix to the kernel types so it is clear which is being used. Signed-off-by: Daniel Wagner <wagi@kernel.org>
libnvme_fabrics_config is going to be removed from the library API. Add the missing common command-line options to nvmf_args and introduce a central function that sets fabrics options in nvmf_context. This reduces the usage of libnvme_fabrics_options to a single place. Signed-off-by: Daniel Wagner <wagi@kernel.org>
The controller is created from the provided fabrics context, which contains all the information required to fully construct it. The libnvmf_add_ctrl function should only perform the connect call. Signed-off-by: Daniel Wagner <wagi@kernel.org>
The typedefs are defined in the public fabrics.h. Minimize the dependency on fabrics in the private.h header by using the concrete types for the argument types. This prepares the stage to drop the include. Signed-off-by: Daniel Wagner <wagi@kernel.org>
Simplify maintainers' workflow by printing the symbols to add in the correct format so they can be copied and pasted directly. Signed-off-by: Daniel Wagner <wagi@kernel.org>
Retire struct libnvme_fabrics_config from the public API and replace it with an anonymous struct accessed via getter/setters. This makes it possible to extend the API without breaking the ABI. Signed-off-by: Daniel Wagner <wagi@kernel.org>
The tls_configured_key_id should also be handled by the merge/update logic. Signed-off-by: Daniel Wagner <wagi@kernel.org>
Copolit insist on correct spelling. Signed-off-by: Daniel Wagner <wagi@kernel.org>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Move
struct libnvme_fabrics_configinto the private section, so it's not part of the stable API.