[pull] master from linux-nvme:master#219
Merged
Merged
Conversation
Adds Windows ioctl support in ioctl-win.c, and pulls Linux-specific methods into ioctl-linux.c. Signed-off-by: Broc Going <bgoing@micron.com>
When the kernel is built without CONFIG_NVME_MULTIPATH, the iopolicy sysfs attribute is not exposed under the NVMe subsystem hierarchy. In this case, libnvme_subsystem_get_iopolicy() returns NULL. However, nvme-top dereferences the returned pointer without validating it first, leading to a NULL pointer dereference and a resulting SIGSEGV on non- multipath systems. Fix this by validating the return value of nvme_subsystem_get_iopolicy() before dereferencing it. If the helper returns NULL, display "NA" and thus avoid NULL pointer dereference. Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
While building the NVMe topology on multipath systems, libnvme constructs the namespace path list by scanning the /sys/block/<nshead-dev>/multipath directory to discover all paths associated with a namespace head node. The scandir() helper allocates an array of struct dirent entries using malloc(), and it is the caller's responsibility to free the allocated array once it is no longer needed. libnvme_subsystem_set_ns_path() invokes scandir() to discover namespace paths and populate head->paths, but it never frees the allocated dirent array, resulting in a memory leak. Fix this by annotating the struct dirents instance with __cleanup_dirents, ensuring that the allocated dirent array is automatically freed when leaving scope. Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
Specifies utf-8 encoding when reading text from files to avoid errors seen when reading with default encoding on Windows. Signed-off-by: Broc Going <bgoing@micron.com>
Update to use memset instead of bzero. Memset is a C standard and is compatible across platforms, while bzero is deprecated. Signed-off-by: Broc Going <bgoing@micron.com>
The width of unsigned long can be 32 or 64 bits, depending on the platform. Update plugins that were using unsigned long for casts to use cross-platform compatible uintptr_t and __u64 types. Signed-off-by: Broc Going <bgoing@micron.com>
The ocp code was using fstat to check S_ISBLK to determine if the device handle was a namespace device before calling libnvme_get_nsid. Update to use the built-in libnvme_transport_handle_is_ns check for cross-platform compatibility. Signed-off-by: Broc Going <bgoing@micron.com>
MinGW on Windows requires gnu_printf formatting to match the C99 formats used by printf on Linux by default. Updates __libnvme_msg to use gnu_printf formatting when building on Windows with MinGW for compatibility. Signed-off-by: Broc Going <bgoing@micron.com>
Adds random_bytes helper that uses Windows' bcrypt support if available and otherwise defaults to using /dev/urandom. Updates libnvme_random_uuid to use the new helper. Signed-off-by: Broc Going <bgoing@micron.com>
Remove include of linux/fs.h from fdp.c. Nothing in fdp.c relies on linux/fs.h, and removing it allows compilation on Windows. Signed-off-by: Broc Going <bgoing@micron.com>
The nvme top UI consists of two dashboards. The first dashboard displays the subsystem list and allows the user to scroll through available subsystems and select one for further inspection. Pressing ENTER on a selected subsystem opens the second dashboard, which displays detailed topology and subsystem information. When the subsystem list dashboard is initially displayed, the first subsystem entry (row #0) is highlighted by default. However, while the user is navigating through the subsystem list, the highlighted selection may unexpectedly jump back to the first subsystem entry. This happens when nvme top receives a uevent (for block or NVMe), triggering a topology rescan and dashboard refresh. During the redraw operation, nvme-top currently resets the selected entry to the first subsystem in the list, even if the previously selected subsystem still exists after the topology refresh. This results in an inconsistent and disruptive user experience. Fix this by preserving the currently selected subsystem across topology rescans. If the previously highlighted subsystem is still present after rebuilding the topology, continue highlighting the same subsystem instead of resetting the selection to the first entry in the list. Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
table_add_columns_filter() iterates over all possible columns and invokes table_add_column() for each entry. table_add_column() allocates memory for the column and initializes various parameters such as the column name, width, alignment, etc. However, if adding a column fails, table_add_column() returns -ENOMEM to table_add_columns_filter(). In the failure path, table_add_columns_filter() does not free the memory already allocated for previously added columns, resulting in a memory leak. Fix this by unwinding the already added columns and freeing their associated memory when an error occurs. Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
The current table APIs automatically adjust column widths based on the maximum length of the values stored in each column. While this behavior works well for one-shot nvme-cli commands, it causes visual instability in realtime dashboards such as nvme-top. In nvme-top, column values continuously fluctuate as statistics are refreshed. Since column widths are recalculated dynamically, the table layout may shift during redraws, resulting in a cluttered and distracting dashboard view. Extend the table APIs to support fixed-width columns. Introduce a new AUTO_WIDTH macro to preserve the existing auto-adjust behavior, while also allowing callers to explicitly specify a fixed column width when a stable table layout is preferred. This allows realtime dashboards such as nvme-top to maintain a consistent and visually stable layout across screen refreshes. Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
The table APIs now support both fixed-width and automatically sized columns. With the updated API, callers must explicitly mark columns with AUTO_WIDTH when automatic width adjustment is desired. For one-shot commands such as, "show-topology --output-format=tabular", automatic column sizing remains the preferred behavior, since it allows the table layout to adapt to the displayed content. Update the show-topology tabular output code to annotate all columns with AUTO_WIDTH so that column widths continue to be automatically derived from the row values. Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
nvme-top currently uses the table APIs with automatic column width adjustment, where column widths are dynamically resized based on the length of the values displayed under each column. Since nvme-top renders a realtime dashboard, the displayed statistics continuously fluctuate across refresh intervals. As a result, column widths may repeatedly expand or shrink during screen updates, causing the table layout to shift frequently. This leads to a visually unstable dashboard and makes it harder to monitor the output in realtime. Now that the table APIs support fixed-width columns, update the nvme-top tables to use fixed column widths in order to maintain a stable and consistent dashboard layout across screen refreshes. Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
Add config_meson_nofabrics() to scripts/build.sh that configures nvme-cli with all optional dependencies disabled (fabrics, mi, python, json-c, libkmod, openssl, keyutils), and a corresponding build-nofabrics job in build.yml that runs the unit test suite. Signed-off-by: Martin Belanger <martin.belanger@dell.com> Assisted-by: Claude:claude-sonnet-4-6 [Claude Code]
Register a named valgrind test setup in meson.build so that unit tests can be run under valgrind without remembering wrapper flags: meson test -C .build --setup valgrind The setup uses --leak-check=full and treats any leak as a failure via --error-exitcode=1. The timeout multiplier is set to 5 to account for valgrind's runtime overhead. Document the setup in TESTING.md under a new "Memory Testing" section, including known limitations for shell script and Python tests that are structurally incompatible with exe_wrapper instrumentation. Signed-off-by: Martin Belanger <martin.belanger@dell.com> Assisted-by: Claude:claude-sonnet-4-6 [Claude Code]
Register a named asanubsan test setup in meson.build for running unit tests with AddressSanitizer and UndefinedBehaviourSanitizer enabled. Unlike Valgrind, sanitizers are compiled into the binaries and require a dedicated build directory: meson setup .build-asanubsan -Db_sanitize=address,undefined meson compile -C .build-asanubsan meson test -C .build-asanubsan --setup asanubsan ASan detects heap overflows, use-after-free, and memory leaks. UBSan catches C undefined behaviour such as signed integer overflow and misaligned pointer access. Together they cover a broader range of issues than either tool alone. Document the setup in TESTING.md under the existing "Memory Testing" section. Signed-off-by: Martin Belanger <martin.belanger@dell.com> Assisted-by: Claude:claude-sonnet-4-6 [Claude Code]
Add a valgrind.supp suppression file to silence known false positives
so that meson test --setup valgrind exits cleanly. Three categories are
suppressed: bash interpreter internals (shell-script-wrapped tests),
CPython interpreter internals (Python test scripts), and SWIG module
teardown (libnvme Python bindings cleanup). Any failure that survives
suppression is a real issue in our C code.
The system python3.supp is loaded in addition when present, detected at
configure time via fs.is_file().
Align scripts/build.sh with the named meson test setups:
-x now uses --setup valgrind instead of --wrapper valgrind
-s (new) enables ASan+UBSan: passes -Db_sanitize=address,undefined at
configure time and runs --setup asanubsan
Signed-off-by: Martin Belanger <martin.belanger@dell.com>
Assisted-by: Claude:claude-sonnet-4-6 [Claude Code]
nvme_hostid_from_hostnqn() returns a freshly strdup'd string. Assigning it to the local `hostid` pointer and then calling strdup(hostid) again loses the original pointer, leaking the first allocation. Fix by transferring ownership directly: when the caller supplies a hostid, strdup it into h->hostid. Otherwise derive it from the hostnqn and assign directly. __libnvme_free_host() already frees h->hostid unconditionally, so both paths are correct. Found by: meson test --setup valgrind Signed-off-by: Martin Belanger <martin.belanger@dell.com> Assisted-by: Claude:claude-sonnet-4-6 [Claude Code]
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )