Remove global headers to avoid spaghetti dependencies - #3814
Merged
Conversation
Rename to reflect what the module actually does: parse generic log-page structures field-by-field for print/JSON output, rather than being a generic utility grab-bag. Used by both the micron and ocp plugins, so it stays shared under src/. Signed-off-by: Daniel Wagner <dwagner@suse.com>
The header file was renamed to avoid clashing with the json-c libray header file. This update missed to update the C file. Signed-off-by: Daniel Wagner <dwagner@suse.com>
There is no point in abstracting the libnvme API unnecessarily. Remove unused helpers and inline the code into the call site. Signed-off-by: Daniel Wagner <dwagner@suse.com>
There is no point in abstracting the libnvme API unnecessarily. A handful of callers across src/ and plugins/ don't justify a shared header wrapper. Signed-off-by: Daniel Wagner <dwagner@suse.com>
Unlike the other survivors in nvme-cmds.h, these two add no logic beyond forwarding to nvme_init_identify_ctrl/_ns() + an admin passthru, so they don't earn a shared header wrapper regardless of call count. Inline both at every one of their ~80 call sites across src/ and plugins/. Signed-off-by: Daniel Wagner <dwagner@suse.com>
dwsuse
changed the title
There was a problem hiding this comment.
Pull request overview
This PR advances the “include what you use” goal by removing the old umbrella headers (src/nvme.h, src/types.h) and relocating declarations/utility code into more focused headers and compilation units. It also updates many call sites to depend directly on libnvme APIs instead of local wrapper helpers.
Changes:
- Removed global headers (
src/nvme.h,src/types.h) and redistributed their declarations into targeted headers (e.g.,src/global-ctx.h,src/plugin.h,src/args.h,src/nvme-print.h). - Replaced many
nvme_*helper wrappers with directnvme_init_*+libnvme_exec_*/libnvme_get_log*usage across core and plugins. - Added new modules for global context/device opening (
src/global-ctx.[ch]), JSON helpers (src/nvme-json.c), and field parsing (src/field-parser.[ch]), plus build integration via Meson.
Reviewed changes
Copilot reviewed 121 out of 122 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types.h | Removed legacy get-log argument type header. |
| src/plugin.h | Added missing includes and moved extension/identify-control declarations here. |
| src/plugin.c | Updated include dependencies to avoid nvme.h. |
| src/nvme.h | Removed legacy global umbrella header. |
| src/nvme-rpmb.c | Replaced wrapper identify calls with passthru cmd init + exec. |
| src/nvme-print.h | Removed nvme.h/types.h dependency; moved topology helpers; updated nvme_show_log API. |
| src/nvme-print.c | Dropped nvme.h include; rewired verbose log printing to new nvme_show_log implementation. |
| src/nvme-print-stdout.c | Removed per-backend .log handler and associated dispatcher. |
| src/nvme-print-stdout-top.c | Updated includes to use cleanup.h/global-ctx.h. |
| src/nvme-print-json.c | Dropped nvme.h include; depends on cleanup.h. |
| src/nvme-print-binary.c | Added explicit libnvme include for types. |
| src/nvme-pci-ids-linux.c | Switched from <nvme/types.h> to <libnvme.h>. |
| src/nvme-models.c | Updated includes to avoid nvme.h, add <libnvme.h>. |
| src/nvme-json.c | New JSON utility implementation (string-backed numeric serialization, helpers). |
| src/nvme-cmds.h | Removed large set of local inline wrappers; kept small subset. |
| src/nvme-cmds.c | Removed local attach/detach controller wrapper implementation. |
| src/meson.build | Updated source list: add global-ctx.c, field-parser.c, nvme-json.c; remove old sources. |
| src/global-ctx.h | New API for global ctx creation and open helpers. |
| src/global-ctx.c | New implementation for ctx creation, option parsing, open helpers. |
| src/field-parser.h | Renamed/repurposed header and updated documentation + includes. |
| src/field-parser.c | Updated include and file/module documentation. |
| src/fabrics.c | Replaced nvme.h include with global-ctx.h. |
| src/dashboard.c | Added <libnvme.h> include explicitly. |
| src/config-create.c | Added global-ctx.h include. |
| src/config-convert.c | Added global-ctx.h include. |
| src/cleanup.h | Added __cleanup_nvme_transport_handle helper. |
| src/args.h | Moved arg-related externs and prototypes here; added argconfig.h include. |
| src/args.c | Implemented parse_args/format helpers and moved extern strings here. |
| src/argconfig.c | Switched from local libnvme_strerror decl to <libnvme.h>. |
| shared/time-util.h | Added <sys/time.h> and shr_elapsed_utime prototype. |
| shared/time-util.c | Added shr_elapsed_utime implementation. |
| shared/string-util.h | Added fallback strsep() implementation for platforms missing it. |
| plugins/zns/zns.c | Dropped nvme.h; updated identify/get-log calls; switched to shr_elapsed_utime. |
| plugins/ymtc/ymtc-nvme.c | Dropped nvme.h; updated identify/get-log calls; uses global-ctx.h. |
| plugins/wdc/wdc-utils.c | Dropped nvme.h; updated identify calls to passthru pattern. |
| plugins/wdc/wdc-nvme.c | Dropped nvme.h; updated multiple passthru/get-log and get-features call patterns. |
| plugins/wdc/wdc-nvme-cmds.h | Added #pragma once and plugin.h include. |
| plugins/virtium/virtium-nvme.c | Dropped nvme.h; updated identify/get-log calls; uses global-ctx.h. |
| plugins/utils/utils.c | Dropped nvme.h; include plugin.h. |
| plugins/utils/command-metadata.c | Dropped nvme.h; include args.h/plugin.h. |
| plugins/transcend/transcend-nvme.c | Dropped nvme.h; include cleanup.h/global-ctx.h. |
| plugins/toshiba/toshiba-nvme.c | Dropped nvme.h; updated get-log calls to passthru pattern. |
| plugins/ssstc/ssstc-nvme.c | Dropped nvme.h; include cleanup.h/global-ctx.h. |
| plugins/solidigm/solidigm-workload-tracker.h | Added #pragma once and plugin.h include. |
| plugins/solidigm/solidigm-workload-tracker.c | Added explicit includes (unistd.h, <libnvme.h>, plugin/global-ctx). |
| plugins/solidigm/solidigm-util.h | Dropped nvme.h; added std + <libnvme.h> includes. |
| plugins/solidigm/solidigm-util.c | Updated identify UUID list call to passthru pattern. |
| plugins/solidigm/solidigm-temp-stats.h | Added #pragma once and plugin.h include. |
| plugins/solidigm/solidigm-temp-stats.c | Added explicit <libnvme.h> and plugin/global-ctx includes. |
| plugins/solidigm/solidigm-telemetry/telemetry-log.h | Dropped nvme.h; added <libnvme.h> + <stdint.h>. |
| plugins/solidigm/solidigm-telemetry.h | Added #pragma once and plugin.h include. |
| plugins/solidigm/solidigm-telemetry.c | Dropped nvme.h; include cleanup.h/global-ctx.h. |
| plugins/solidigm/solidigm-smart.h | Added #pragma once and plugin.h include. |
| plugins/solidigm/solidigm-smart.c | Dropped nvme.h; include cleanup.h/global-ctx.h. |
| plugins/solidigm/solidigm-ocp-version.h | Added #pragma once and plugin.h include. |
| plugins/solidigm/solidigm-ocp-version.c | Dropped nvme.h; include args.h/plugin.h. |
| plugins/solidigm/solidigm-nvme.c | Dropped nvme.h; include plugin.h. |
| plugins/solidigm/solidigm-market-log.h | Added #pragma once and plugin.h include. |
| plugins/solidigm/solidigm-market-log.c | Dropped nvme.h; include cleanup.h/global-ctx.h. |
| plugins/solidigm/solidigm-log-page-dir.h | Added plugin.h include for shared command/plugin types. |
| plugins/solidigm/solidigm-log-page-dir.c | Added <libnvme.h> and plugin/global-ctx includes; updated UUID list identify call. |
| plugins/solidigm/solidigm-latency-tracking.h | Added #pragma once and plugin.h include. |
| plugins/solidigm/solidigm-latency-tracking.c | Dropped nvme.h; include cleanup.h/global-ctx.h. |
| plugins/solidigm/solidigm-internal-logs.h | Added #pragma once and plugin.h include. |
| plugins/solidigm/solidigm-internal-logs.c | Dropped nvme.h; include cleanup.h/global-ctx.h; added <sys/stat.h> and passthru conversions. |
| plugins/solidigm/solidigm-get-drive-info.h | Added #pragma once and plugin.h include. |
| plugins/solidigm/solidigm-get-drive-info.c | Added explicit <libnvme.h> and plugin/global-ctx includes. |
| plugins/solidigm/solidigm-garbage-collection.h | Added #pragma once and plugin.h include. |
| plugins/solidigm/solidigm-garbage-collection.c | Dropped nvme.h; include cleanup.h/global-ctx.h. |
| plugins/shannon/shannon-nvme.c | Dropped nvme.h; include cleanup.h/global-ctx.h; updated get-log calls. |
| plugins/sed/sed.c | Dropped nvme.h; include plugin.h/cleanup.h/global-ctx.h. |
| plugins/seagate/seagate-nvme.c | Dropped nvme.h; include cleanup.h/global-ctx.h; updated identify/get-log calls. |
| plugins/scaleflux/sfx-nvme.c | Dropped nvme.h; include global-ctx.h; updated identify/get-log calls. |
| plugins/sandisk/sandisk-utils.c | Dropped nvme.h; updated identify calls to passthru pattern. |
| plugins/sandisk/sandisk-nvme.c | Dropped nvme.h; include global-ctx.h; updated identify calls. |
| plugins/registry/registry-nvme.c | Dropped nvme.h; include plugin.h/global-ctx.h. |
| plugins/ocp/ocp-utils.h | Dropped nvme.h; added <libnvme.h>/<stdbool.h>. |
| plugins/ocp/ocp-utils.c | Dropped types.h; updated UUID identify call to passthru pattern. |
| plugins/ocp/ocp-telemetry-decode.h | Dropped nvme.h/utils.h; include <libnvme.h> and field-parser.h. |
| plugins/ocp/ocp-telemetry-decode.c | Dropped nvme.h include. |
| plugins/ocp/ocp-smart-extended-log.h | Added plugin.h include. |
| plugins/ocp/ocp-smart-extended-log.c | Added <libnvme.h> and plugin/global-ctx includes. |
| plugins/ocp/ocp-print.c | Added explicit <libnvme.h> include. |
| plugins/ocp/ocp-print-stdout.c | Added explicit <libnvme.h> include. |
| plugins/ocp/ocp-print-json.c | Added <libnvme.h> + cleanup.h include. |
| plugins/ocp/ocp-print-binary.c | Added explicit <libnvme.h> include. |
| plugins/ocp/ocp-nvme.c | Dropped nvme.h; include cleanup.h/global-ctx.h; updated telemetry get-log calls. |
| plugins/ocp/ocp-hardware-component-log.h | Added plugin.h include. |
| plugins/ocp/ocp-hardware-component-log.c | Added <libnvme.h> + plugin/global-ctx includes. |
| plugins/ocp/ocp-fw-activation-history.h | Added plugin.h include. |
| plugins/ocp/ocp-fw-activation-history.c | Added <libnvme.h> + plugin/global-ctx includes. |
| plugins/ocp/ocp-clear-features.h | Added #pragma once and plugin.h include. |
| plugins/ocp/ocp-clear-features.c | Added <libnvme.h> + plugin/global-ctx includes. |
| plugins/netapp/netapp-nvme.c | Dropped nvme.h; updated identify/desc-list calls to passthru pattern. |
| plugins/nbft/nbft-plugin.c | Dropped nvme.h; include plugin.h/cleanup.h/global-ctx.h. |
| plugins/micron/micron-nvme.c | Dropped nvme.h/utils.h; include plugin.h/global-ctx.h/field-parser.h; updated many passthru/get-log calls. |
| plugins/memblaze/memblaze-smart-log-add-x.h | Dropped types.h; include <nvme/nvme-types.h> and plugin.h. |
| plugins/memblaze/memblaze-smart-log-add-x.c | Dropped nvme.h; include global-ctx.h. |
| plugins/memblaze/memblaze-nvme.c | Dropped nvme.h; include cleanup.h/global-ctx.h; updated identify/get-log/get-features patterns. |
| plugins/lm/lm-print.h | Dropped nvme.h; include args.h. |
| plugins/lm/lm-print.c | Added explicit <libnvme.h> include. |
| plugins/lm/lm-print-stdout.c | Added explicit <libnvme.h> include. |
| plugins/lm/lm-print-json.c | Added explicit <libnvme.h> include. |
| plugins/lm/lm-print-binary.c | Added explicit <libnvme.h> include and nvme-print.h include. |
| plugins/lm/lm-nvme.c | Dropped nvme.h; include global-ctx.h. |
| plugins/keys/keys-plugin.c | Dropped nvme.h; include plugin.h/cleanup.h/global-ctx.h. |
| plugins/intel/intel-nvme.c | Dropped nvme.h; include cleanup.h/global-ctx.h; updated identify calls. |
| plugins/inspur/inspur-nvme.c | Dropped nvme.h; include cleanup.h/global-ctx.h. |
| plugins/innogrit/innogrit-nvme.c | Dropped nvme.h; include plugin.h/global-ctx.h; updated get-log calls. |
| plugins/ibm/ibm-nvme.c | Dropped nvme.h; include <libnvme.h>/cleanup.h/global-ctx.h. |
| plugins/huawei/huawei-nvme.c | Dropped nvme.h; include cleanup.h/global-ctx.h; updated identify calls. |
| plugins/feat/feat-nvme.c | Dropped nvme.h; include <libnvme.h>/cleanup.h/global-ctx.h. |
| plugins/fdp/fdp.c | Dropped nvme.h; include plugin.h/cleanup.h/global-ctx.h; updated get-log calls. |
| plugins/exclusion/exclusion-nvme.c | Dropped nvme.h; include plugin.h/global-ctx.h. |
| plugins/dera/dera-nvme.c | Dropped nvme.h; include cleanup.h/global-ctx.h. |
| plugins/config/config-nvme.c | Dropped nvme.h; include plugin.h. |
| plugins/amzn/amzn-nvme.c | Dropped nvme.h; include cleanup.h/global-ctx.h; updated identify calls. |
| plugins/nvidia/nvidia-nvme.c | Dropped nvme.h include. |
| plugins/mangoboost/mangoboost-nvme.c | Dropped nvme.h include. |
| plugins/dell/dell-nvme.c | Dropped nvme.h include. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Like nvme_identify_ctrl/_ns before it, this wrapper adds no logic beyond nvme_init_get_log + libnvme_get_log, so it doesn't earn a shared header wrapper. Inline it at all 10 call sites across plugins/. Signed-off-by: Daniel Wagner <dwagner@suse.com>
nvme_show_log took struct nvme_get_log_args just o hand it to print_ops.stdout_log for demultiplexing by log ID. Move the log-ID switch out of stdout_log and into nvme_show_log itself, calling the existing per-log-page nvme_show_*_log wrappers directly instead of a format-specific callback. Each of those already dispatches correctly across stdout/json/binary, so print_ops.log and stdout_log become dead code and are removed, and nvme_show_log can take just the scalar fields printing actually needs instead of the whole struct. struct nvme_get_log_args is now only used in nvme.c for building real libnvme_get_log() calls, so move it there and delete types.h Signed-off-by: Daniel Wagner <dwagner@suse.com>
This header files contains random function and definition. Move them to a new home where they are group with other matching functions and definition. Signed-off-by: Daniel Wagner <dwagner@suse.com>
Having a global header containing includes unnecessarily couples multiple components together, making separation and compartmentalization harder. Therefore, this header is being removed. Signed-off-by: Daniel Wagner <dwagner@suse.com>
open_exclusive() computes O_RDONLY | O_EXCL depending on --force, but libnvme_open had no flags parameter and unconditionally opened the device with O_RDONLY. As a result --force/ignore_exclusive never affected the underlying open(2) call, and the exclusive-open check added for format/write could never detect a namespace already in use. Also fix open_exclusive to return the real error from get_transport_handle instead of collapsing it to -ENXIO. The format and write command handlers key off -EBUSY specifically to print the "use --force" hint, which could never trigger while the real error was being masked. Signed-off-by: Daniel Wagner <dwagner@suse.com>
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.
Reorganize the declarations and definitions in the different global headers by grouping them together according to their functionality into the matching header files. The goal here is to get rid of these global headers, which just randomly tie things together.
The object is "include what you use"