Refactoring and reorganize the public headers#3116
Merged
igaw merged 29 commits intolinux-nvme:masterfrom Mar 4, 2026
Merged
Conversation
The libnvme.h header is not a local header file, thus use the standard lookup <> for external headers. Signed-off-by: Daniel Wagner <wagi@kernel.org>
Use the set feature defines instead of the identify. Signed-off-by: Daniel Wagner <wagi@kernel.org>
igaw
force-pushed
the
header-mania
branch
3 times, most recently
from
d239f84 to
dcf4423
Compare
Add a simple test which ensures that all headers are self contained and don't have an undocumented dependency. Signed-off-by: Daniel Wagner <wagi@kernel.org>
Unify the header includes. Use the libnvme.h or/and libnvme-mi.h everywhere instead indiviual headers. This makes the refactoring of the headers simpler (splitting them into smaller ones). Signed-off-by: Daniel Wagner <wagi@kernel.org>
clang reports these are unused, thus remove them from the include list. Signed-off-by: Daniel Wagner <wagi@kernel.org>
Use nvme_ns_rescan which wraps the IOCTL. This in preparation to make the low level code private in libnvme. Signed-off-by: Daniel Wagner <wagi@kernel.org>
igaw
force-pushed
the
header-mania
branch
3 times, most recently
from
4fdd387 to
8dd8dd3
Compare
igaw
changed the title
There was a problem hiding this comment.
Pull request overview
Refactors and reorganizes libnvme public headers to reduce include coupling and enforce a cleaner include hierarchy across nvme-cli plugins/tests.
Changes:
- Switch many users from local
"libnvme.h"includes to system-style<libnvme.h>and adjust include ordering. - Split/relocate libnvme APIs into more focused headers (e.g., new
nvme/lib.h,nvme/lib-types.h,nvme/cmds.[ch]) and removenvme/log.h. - Add build-time header self-sufficiency tests to ensure each public header can be included standalone.
Reviewed changes
Copilot reviewed 99 out of 101 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| util/cleanup.h | Update URI cleanup helper to use nvmf_free_uri() |
| plugins/zns/zns.c | Switch to <libnvme.h> include style |
| plugins/ymtc/ymtc-nvme.c | Switch to <libnvme.h> include style |
| plugins/wdc/wdc-utils.c | Switch to <libnvme.h> include style |
| plugins/wdc/wdc-nvme.c | Switch to <libnvme.h> include style |
| plugins/virtium/virtium-nvme.c | Switch to <libnvme.h> include style |
| plugins/transcend/transcend-nvme.c | Switch to <libnvme.h> include style |
| plugins/toshiba/toshiba-nvme.c | Switch to <libnvme.h> include style |
| plugins/ssstc/ssstc-nvme.c | Switch to <libnvme.h> include style |
| plugins/solidigm/solidigm-telemetry/telemetry-log.h | Switch to <libnvme.h> include style |
| plugins/solidigm/solidigm-telemetry.c | Switch to <libnvme.h> include style |
| plugins/solidigm/solidigm-smart.c | Switch to <libnvme.h> include style |
| plugins/solidigm/solidigm-market-log.c | Switch to <libnvme.h> include style |
| plugins/solidigm/solidigm-latency-tracking.c | Switch to <libnvme.h> include style |
| plugins/solidigm/solidigm-internal-logs.c | Switch to <libnvme.h> include style |
| plugins/solidigm/solidigm-garbage-collection.c | Switch to <libnvme.h> include style |
| plugins/shannon/shannon-nvme.c | Switch to <libnvme.h> include style |
| plugins/sed/sed.c | Switch to <libnvme.h> include style |
| plugins/seagate/seagate-nvme.c | Switch to <libnvme.h> include style |
| plugins/scaleflux/sfx-nvme.c | Add missing system include(s) and switch to <libnvme.h> |
| plugins/sandisk/sandisk-utils.c | Switch to <libnvme.h> include style |
| plugins/sandisk/sandisk-nvme.c | Switch to <libnvme.h> include style |
| plugins/ocp/ocp-telemetry-decode.c | Switch to <libnvme.h> include style |
| plugins/ocp/ocp-nvme.c | Switch to <libnvme.h> include style |
| plugins/ocp/ocp-fw-activation-history.h | Include formatting/ordering updates |
| plugins/nvidia/nvidia-nvme.c | Switch to <libnvme.h> include style |
| plugins/netapp/netapp-nvme.c | Switch to <libnvme.h> include style |
| plugins/nbft/nbft-plugin.c | Include formatting/ordering updates |
| plugins/micron/micron-nvme.c | Switch to <libnvme.h> include style |
| plugins/memblaze/memblaze-nvme.c | Switch to <libnvme.h> include style |
| plugins/mangoboost/mangoboost-nvme.c | Switch to <libnvme.h> include style |
| plugins/lm/lm-print.h | Switch to <libnvme.h> include style |
| plugins/lm/lm-nvme.c | Switch to <libnvme.h> include style |
| plugins/intel/intel-nvme.c | Switch to <libnvme.h> include style |
| plugins/inspur/inspur-nvme.c | Switch to <libnvme.h> include style |
| plugins/innogrit/innogrit-nvme.c | Switch to <libnvme.h> include style |
| plugins/huawei/huawei-nvme.c | Switch to <libnvme.h> include style |
| plugins/fdp/fdp.c | Switch to <libnvme.h> include style |
| plugins/dera/dera-nvme.c | Switch to <libnvme.h> include style |
| plugins/dell/dell-nvme.c | Switch to <libnvme.h> include style |
| plugins/dapustor/dapustor-nvme.c | Switch to <libnvme.h> include style |
| plugins/amzn/amzn-nvme.c | Switch to <libnvme.h> include style |
| nvme.c | Use nvme_ns_rescan() helper and add required includes |
| nvme-rpmb.c | Switch to <libnvme.h> include style |
| nvme-print.c | Switch to <libnvme.h> include style |
| nvme-print-stdout.c | Switch to <libnvme.h> include style |
| nvme-print-json.c | Switch to <libnvme.h> include style |
| libnvme/test/zns.c | Update tests to include <libnvme.h>/internal private header |
| libnvme/test/uriparser.c | Rename URI free helper call to nvmf_free_uri() |
| libnvme/test/test.c | Update tests to include <libnvme.h>/internal private header |
| libnvme/test/test-util.c | Update test includes to use <libnvme.h> and private header |
| libnvme/test/test-header.c.in | Add template for header self-sufficiency test |
| libnvme/test/nbft/nbft-dump.c | Switch to <libnvme.h> include style |
| libnvme/test/mi.c | Switch to unified global ctx create/free APIs |
| libnvme/test/mi-mctp.c | Switch to unified global ctx create/free APIs + include fixes |
| libnvme/test/meson.build | Use test dep where needed and add header self-sufficiency tests |
| libnvme/test/ioctl/zns.c | Simplify includes; rely on <libnvme.h> |
| libnvme/test/ioctl/mock.c | Switch to <libnvme.h> and adjust private header include |
| libnvme/test/ioctl/misc.c | Simplify includes; rely on <libnvme.h> |
| libnvme/test/ioctl/logs.c | Simplify includes; rely on <libnvme.h> |
| libnvme/src/nvme/util.h | Convert to #pragma once and move APIs out to other headers |
| libnvme/src/nvme/util.c | Adjust includes after header refactor |
| libnvme/src/nvme/types.h | Convert to #pragma once and relocate helper macros/inlines |
| libnvme/src/nvme/tree.h | Convert to #pragma once and update include dependencies |
| libnvme/src/nvme/tree.c | Adjust includes and move global ctx logic out |
| libnvme/src/nvme/private.h | Convert to #pragma once, add ioctl/uring macro defs, and move internal decls |
| libnvme/src/nvme/no-json.c | Switch to <libnvme.h> include style |
| libnvme/src/nvme/nbft.h | Convert to #pragma once and update include dependencies |
| libnvme/src/nvme/nbft.c | Switch to <libnvme.h> include style and adjust includes |
| libnvme/src/nvme/mi.h | Convert to #pragma once and remove duplicated APIs |
| libnvme/src/nvme/mi.c | Switch to <libnvme.h>/<libnvme-mi.h> include style and remove duplicated ctx APIs |
| libnvme/src/nvme/mi-mctp.c | Switch to <libnvme.h>/<libnvme-mi.h> include style and add needed includes |
| libnvme/src/nvme/log.h | Remove header (APIs relocated) |
| libnvme/src/nvme/log.c | Switch to <libnvme.h> include style and rename logging setter API |
| libnvme/src/nvme/linux.h | Convert to #pragma once and slim API surface |
| libnvme/src/nvme/linux.c | Move many APIs out; switch to <libnvme.h> include style |
| libnvme/src/nvme/lib.h | New public header for global ctx, logging, and transport handle APIs |
| libnvme/src/nvme/lib.c | New implementation file for global ctx and transport-handle open/close logic |
| libnvme/src/nvme/lib-types.h | New public header for shared public structs (nvme_passthru_cmd, nvme_uring_cmd) |
| libnvme/src/nvme/json.c | Switch to <libnvme.h> include style and adjust includes |
| libnvme/src/nvme/ioctl.c | Switch to <libnvme.h> include style and adjust includes |
| libnvme/src/nvme/filters.c | Switch to <libnvme.h> include style and adjust includes |
| libnvme/src/nvme/fabrics.h | Rename nvme_free_uri() API to nvmf_free_uri() |
| libnvme/src/nvme/fabrics.c | Switch to <libnvme.h> include style and rename URI free calls |
| libnvme/src/nvme/crc32.h | Convert to #pragma once and adjust license header |
| libnvme/src/nvme/crc32.c | Add SPDX license tag |
| libnvme/src/nvme/cmds.c | New implementation file for linux/cmd helper APIs previously elsewhere |
| libnvme/src/nvme/cleanup.h | Update URI cleanup helper to use nvmf_free_uri() |
| libnvme/src/nvme/base64.h | Convert to #pragma once and adjust license header |
| libnvme/src/nvme/base64.c | Adjust SPDX tag and includes |
| libnvme/src/meson.build | Update libnvme sources and installed headers list |
| libnvme/src/libnvme.ld | Update exported symbol list to match header/API refactor |
| libnvme/src/libnvme.h | Update umbrella header include list (new split headers, remove log.h) |
| libnvme/src/libnvme-mi.h | Update MI umbrella header includes |
| libnvme/libnvme/nvme.i | Switch SWIG interface to include <libnvme.h> and rename logging setter usage |
| libnvme/examples/mi-mctp.c | Switch to unified global ctx create/free APIs |
| libnvme/examples/mi-mctp-csi-test.c | Switch to unified global ctx create/free APIs |
| libnvme/examples/mi-mctp-ae.c | Switch to unified global ctx create/free APIs |
| libnvme/doc/meson.build | Remove log.h from documented API headers list |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Don't expose these low level defines anymore. The library should provide all necessary functions/helpers. Signed-off-by: Daniel Wagner <wagi@kernel.org>
These data structs are libnvme APIs and just match the Linux APIs. libnvme should not expose directly the Linux data structs. Signed-off-by: Daniel Wagner <wagi@kernel.org>
Split the ioctl APIs from the commands API. While at it, also remove a few of the unnecessary header dependencies. The aim here is to make the ioctl header as small as possible. This will make any porting attempts simpler. Signed-off-by: Daniel Wagner <wagi@kernel.org>
Sort the install_headers list alphabetically to match the convention. Signed-off-by: Daniel Wagner <wagi@kernel.org>
These function/helpers are nvme commands and are not Linux specific. Signed-off-by: Daniel Wagner <wagi@kernel.org>
Forward declare the three data structs and thus avoid to include nvme/ioctl.h and nvme/types.h. This reduces the inter dependencies between the headers and simplifies any porting attempts. Signed-off-by: Daniel Wagner <wagi@kernel.org>
Move the nvme specification parts of utils to either cmds.h or types.h and reduce the number of include dependencies. Signed-off-by: Daniel Wagner <wagi@kernel.org>
To reduce complexity and dependencies between the header, include the types.h header and drop the rest. Signed-off-by: Daniel Wagner <wagi@kernel.org>
Include nvme/types.h directly instead via nvme/util.h Signed-off-by: Daniel Wagner <wagi@kernel.org>
The crc.[ch] are missing the spdx tag. Signed-off-by: Daniel Wagner <wagi@kernel.org>
The code is a copy from the nvme-cli project which was licesenced under the GPL-2.0-or-later license. The library is LGPL-2.1-or-later. Hannes agreed to relicense this code. Signed-off-by: Hannes Reinecke <hare@suse.de> Signed-off-by: Daniel Wagner <wagi@kernel.org>
The dependency on including the root object has been removed, thus drop the include. Signed-off-by: Daniel Wagner <wagi@kernel.org>
The tree.h included already does include types.h. Thus it's possible to drop this include. Signed-off-by: Daniel Wagner <wagi@kernel.org>
Refactor all library specific definitions and declarations into two headers. lib-types.h contains the forward declarations for all types used in libnvme. lib.h contains the interfaces and declarations that need to interact with the library. Signed-off-by: Daniel Wagner <wagi@kernel.org>
These function are only used internally, thus move it to private and thus reduce the API size and dependencies a bit further. Signed-off-by: Daniel Wagner <wagi@kernel.org>
The logging is attached to the global context and thus no low level API is needed outside of the library. Thus move it to lib.h. Signed-off-by: Daniel Wagner <wagi@kernel.org>
There is no user left for this interface, thus move it to the private header and reduce the API. Signed-off-by: Daniel Wagner <wagi@kernel.org>
Merge these two function together and avoid code duplication. Signed-off-by: Daniel Wagner <wagi@kernel.org>
This function can be retired from the public API after the libnvme-me merge into the libnvme. Signed-off-by: Daniel Wagner <wagi@kernel.org>
This data structure is used through out the library and there is no point in forward declare it and then have it in ioctl.h. Signed-off-by: Daniel Wagner <wagi@kernel.org>
All fabrics function should use the nvmf_ prefix. Signed-off-by: Daniel Wagner <wagi@kernel.org>
nvme_fw_download_seq() updates data, size, and offset by xfer even though the actual submitted chunk length is min(xfer, size). If xfer > size, size -= xfer underflows (since size is __u32), potentially causing an infinite loop and out-of-bounds reads. Track the chunk size in a variable and advance/decrement by that value instead. Signed-off-by: Daniel Wagner <wagi@kernel.org>
id_ctrl is allocated with __nvme_alloc() but never freed. 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.
Refactoring and reorganize the public headers.