tests: add LD_PRELOAD-mocked fabrics CLI integration tests - #3825
Open
dwsuse wants to merge 2 commits into
Open
Conversation
Add an integration test harness for 'nvme discover', 'nvme connect', and
'nvme connect-all' that requires neither real NVMe-oF hardware nor root
privileges.
libmock_fabrics.c is an LD_PRELOAD shim that intercepts open/write/
ioctl/close on /dev/nvme-fabrics and /dev/nvme<N>, forwarding each
connect and admin passthru command over a Unix socket to a Python-side
IPC server (nvme_fabrics_mock_test.py) that fabricates discovery log
pages, Identify data, and simulated errno failures on demand.
Test coverage includes:
- basic discover/connect and sysfs attribute verification
- connect-all recursive discovery cascades, including multi-hop
Discovery Controller referral chains
- graceful handling of an already-connected (EALREADY) target
- admin-passthru round-tripping custom Identify Controller fields
- the --persistent=[no|auto|force] connection modes, including how
each interacts with a target's self-reported EPCSD (Explicit
Persistent Connection Support for Discovery) flag, both for the
primary discovery controller and for referral hops during
connect-all
- the documented quirk where a --persistent value given as a separate
argv token (rather than glued with '=') is silently dropped
Wire the new libmock_fabrics.so and its test into meson.build, built
and run only when fabrics support is enabled.
Signed-off-by: Daniel Wagner <dwagner@suse.com>
kernel-doc-check greps kernel-doc's stderr for "warning"/"error" to decide pass/fail. When the environment's LANG/LC_* names a locale that isn't actually installed (e.g. a minimal container image with LANG=en_US.UTF-8 but no locale-gen run), perl prints "Setting locale failed" warnings on stderr, which the grep mistakes for a real kernel-doc warning and fails the test. Even though the doc comments are fine. Force LC_ALL=C.UTF-8 before invoking kernel-doc so the check no longer depends on what the calling environment happens to have configured. Signed-off-by: Daniel Wagner <dwagner@suse.com>
There was a problem hiding this comment.
Pull request overview
Adds a new LD_PRELOAD-based mock “fabrics” backend to enable end-to-end CLI integration testing for nvme discover, nvme connect, and nvme connect-all without requiring root or real NVMe-oF hardware. This fits into the existing tests/cli harness by introducing a mock /dev/nvme-fabrics + /dev/nvme<N> implementation and a Python IPC-driven test suite, wired into Meson when fabrics support is enabled.
Changes:
- Introduce a Python integration test runner and IPC server (
nvme_fabrics_mock_test.py) to simulate discovery log pages, Identify data, and errno paths. - Add an LD_PRELOAD shared library shim (
libmock_fabrics.c) that intercepts/dev/nvme-fabricsand/dev/nvme<N>operations and forwards them over a Unix socket to the Python server. - Wire the new shared library and test into
tests/cli/meson.build, and makekernel-doc-checklocale-stable.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| tests/cli/nvme_fabrics_mock_test.py | New Python integration tests + IPC server driving fabrics behavior via LD_PRELOAD. |
| tests/cli/libmock_fabrics.c | New LD_PRELOAD shim intercepting open/write/ioctl/close for mocked fabrics interactions. |
| tests/cli/meson.build | Builds the mock shared library and registers the new CLI integration test under want_fabrics. |
| libnvme/scripts/kernel-doc-check | Forces a stable locale to avoid spurious perl locale warnings being treated as kernel-doc warnings. |
Suppressed comments (2)
tests/cli/nvme_fabrics_mock_test.py:242
- socket.recv() on a stream socket can return fewer bytes than requested. handle_client() reads the fixed-size request header with a single recv(), which can lead to truncated headers and silently dropped requests/hangs under load or on slower systems. Similarly, the payload recv() can be partial.
req_header = conn.recv(_IPC_REQUEST_LEN)
if len(req_header) < _IPC_REQUEST_LEN:
return
tests/cli/libmock_fabrics.c:373
- If the IPC payload read is short (read() returns 0/EOF or an error), the current loop breaks and returns a buffer containing uninitialized bytes. That buffer is later memcpy()'d into the caller's admin-passthru buffer.
while (total_read < resp->data_len) {
ssize_t n = orig_read(ipc_fd, payload + total_read,
resp->data_len - total_read);
if (n <= 0)
break;
total_read += n;
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+232
to
+234
| fr_bytes = b"MOCK1234".ljust(8, b" ")[:8] | ||
| rsvd = b"\x00" * (4096 - len(vid) - len(sn_bytes) - len(mn_bytes) - len(fr_bytes)) | ||
| resp_payload = (vid + sn_bytes + mn_bytes + fr_bytes + rsvd)[:req_len] |
Comment on lines
+479
to
+486
| cmd->result = resp.result; | ||
| if (resp_data) | ||
| memcpy((void *)(uintptr_t)cmd->addr, resp_data, resp.data_len); | ||
| free(resp_data); | ||
| orig_close(ipc_fd); | ||
|
|
||
| return 0; | ||
| } |
Comment on lines
+246
to
+250
| if (strncmp(pathname, "/dev/nvme", 9) || pathname[9] < '0' || pathname[9] > '9') | ||
| return -2; | ||
|
|
||
| instance = atoi(pathname + 9); | ||
| real_fd = orig_open("/dev/null", flags); |
Comment on lines
+101
to
+105
| uint32_t request; /* ioctl request code */ | ||
| uint8_t opcode; /* nvme opcode */ | ||
| uint8_t nsid; | ||
| uint16_t rsvd; | ||
| uint32_t cdw10; |
Comment on lines
+6
to
+11
| # Force a locale that's guaranteed to exist instead of inheriting whatever | ||
| # the environment has set: an unavailable locale (e.g. a LANG naming a | ||
| # locale that isn't installed) makes perl print "Setting locale failed" | ||
| # warnings on stderr, which the grep below then mistakes for a real | ||
| # kernel-doc warning. | ||
| export LC_ALL=C.UTF-8 |
Comment on lines
+353
to
+363
| memset(resp, 0, sizeof(*resp)); | ||
| if (orig_read(ipc_fd, resp, sizeof(*resp)) != (ssize_t)sizeof(*resp)) { | ||
| mock_dbg("failed to read full IPC response header\n"); | ||
| resp->status = -1; | ||
| resp->errno_val = EIO; | ||
| return NULL; | ||
| } | ||
|
|
||
| if (!resp->data_len) | ||
| return NULL; | ||
|
|
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.
Add an integration test harness for 'nvme discover', 'nvme connect', and
'nvme connect-all' that requires neither real NVMe-oF hardware nor root
privileges.
libmock_fabrics.c is an LD_PRELOAD shim that intercepts open/write/
ioctl/close on /dev/nvme-fabrics and /dev/nvme, forwarding each
connect and admin passthru command over a Unix socket to a Python-side
IPC server (nvme_fabrics_mock_test.py) that fabricates discovery log
pages, Identify data, and simulated errno failures on demand.
Test coverage includes:
Discovery Controller referral chains
each interacts with a target's self-reported EPCSD (Explicit
Persistent Connection Support for Discovery) flag, both for the
primary discovery controller and for referral hops during
connect-all
argv token (rather than glued with '=') is silently dropped
Wire the new libmock_fabrics.so and its test into meson.build, built
and run only when fabrics support is enabled.
Signed-off-by: Daniel Wagner dwagner@suse.com