Re: [LTP] testcases/nvme: Add NVMe device discovery and identification test

Re: [LTP] testcases/nvme: Add NVMe device discovery and identification test

[linuxtestproject.agent]

Hi priyama2,

On Wed, 29 Apr 2026, priyama2 wrote:
> testcases/nvme: Add NVMe device discovery and identification test

> This patch introduces the first test case (nvme01) for NVMe device
> testing in LTP. The test verifies: [...]

The commit body describes what the test does and what hardware it was
run on, but doesn't explain *why* this test is being added to LTP —
what gap does it fill, or what problem prompted writing it?

> +TARGETS		= nvme01 nvme02 nvme03 nvme04

nvme02.c, nvme03.c, nvme04.c are not present in this patch, so `make`
will fail. Remove the missing targets (and their explicit rules) from
this patch; add them when those tests land.

> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2024 IBM Corporation
> + * Author: LTP NVMe Test Suite

Copyright year must be 2026 for a newly added file. "Author: LTP NVMe
Test Suite" is not a real author; use your name and email. Also, the
test description must be in a separate /*\ RST block (not merged into
the copyright block).

> +	dir = opendir(NVME_DEV_PATH);
> +	if (!dir) {
> +		tst_brk(TBROK | TERRNO, "Failed to open %s", NVME_DEV_PATH);

Use SAFE_OPENDIR() here (and SAFE_FOPEN() / SAFE_CLOSEDIR() /
SAFE_FCLOSE() elsewhere). These are defined in tst_safe_macros.h and
tst_safe_stdio.h.

> +static int nvme_device_found = 0;
> +static char nvme_dev_name[256];

Both static vars are set in case 0 of run() but never reset. Under
`-i` (iteration), subsequent iterations inherit stale state. Reset
them at the top of run().

> +	.needs_root = 1,

All operations in this test are unprivileged sysfs/devfs reads. Drop
.needs_root.

> +CC		= gcc
> +CFLAGS		= -Wall -O2 -I/opt/ltp/include
> +LDFLAGS		= -L/opt/ltp/lib

This is a hand-rolled standalone Makefile with hardcoded paths. Replace
it with LTP's build system (env_pre.mk + generic_leaf_target.mk). Also
add `nvme` to SUBDIRS in testcases/kernel/device-drivers/Makefile, and
add a runtest entry and .gitignore entry for nvme01.

[...]

Pre-existing issues noticed in the surrounding code (not introduced by
this patch):

- nvme01.c:236 — Stray `// Made with Bob` comment at end of file.
- nvme01.c:58  — `strlen(entry->d_name) == 5` silently skips nvme10+.

---
Note:

Our agent completed the review of the patch. The full review can be
found at: https://github.com/linux-test-project/ltp-agent/actions/runs/25031212578

The agent can sometimes produce false positives although often its
findings are genuine. If you find issues with the review, please
comment this email or ignore the suggestions.

Regards,
LTP AI Reviewer

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] testcases/nvme: Add NVMe device discovery and identification test

[linuxtestproject.agent]

Hi Priya,

On Thu, 30 Apr 2026 15:48:52 +0000, priyama2 wrote:
> testcases/nvme: Add NVMe device discovery and identification test
>
> This test verifies NVMe device discovery and identification by:
> - Detecting NVMe controllers in /dev
> [...]

The commit body describes what the test does but not why it is needed —
what coverage gap does this fill? Also, "Changes in v2:" belongs in the
cover letter, not the commit body; strip it before sending.

> +++ b/testcases/kernel/device-drivers/nvme/nvme01
> new file mode 100755
>  Bin 0 -> 995536 bytes

Do not commit compiled binaries. Remove nvme01 from the patch and add a
.gitignore in the test directory listing it.

> + * Copyright (c) 2024 IBM Corporation

This file is new and added in 2026; use the current year.

> +static int nvme_device_found;
> +static char nvme_dev_name[256];

These statics are set in case 0 of run() but never cleared. With -i,
stale state from a prior iteration persists into subsequent ones. Reset
them at the top of run().

> +	.needs_root = 1,

No operation in this test requires root. All reads from /dev and sysfs
are accessible to unprivileged users. Drop needs_root.

The patch is also missing a runtest entry (e.g. in runtest/drivers) and
a .gitignore entry for the nvme01 binary.

---
Note:

Our agent completed the review of the patch.

The agent can sometimes produce false positives although often its
findings are genuine. If you find issues with the review, please
comment this email or ignore the suggestions.

Regards,
LTP AI Reviewer

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] testcases/nvme: Add NVMe device discovery and identification test

[linuxtestproject.agent]

Hi Priya,

On 2026-04-30, priyama2 <priyama2@linux.ibm.com> wrote:
> [PATCH] testcases/nvme: Add NVMe device discovery and identification test

The subject prefix should match the actual directory path:
testcases/kernel/device-drivers/nvme:

The commit body only describes what the test does (restating the subject).
Please add a sentence explaining why the test is needed, e.g. what gap in
NVMe coverage this fills.

> +/*
> + * Copyright (c) 2024 IBM Corporation

New files added in 2026 must use 2026 as the copyright year.

[...]

> +SUBDIRS		:= acpi \
> +		   ...

testcases/kernel/device-drivers/Makefile has an explicit SUBDIRS list that
does not include nvme. The directory will not be built. Add nvme to the list.

[...]

> +static int nvme_device_found;
> +static char nvme_dev_name[256];

These statics are written in run() case 0 and read in cases 1-3 but are
never reset at the top of run(). With -i, stale values from a previous
iteration carry over. Reset them at the start of run().

[...]

> +	.needs_root = 1,

All operations here (reading /dev, /sys) are world-readable. Remove
.needs_root = 1.

[...]

> +		fp = fopen(class_path, "r");
> +		if (!fp)
> +			continue;
> +		...
> +		fclose(fp);

Use SAFE_FOPEN/SAFE_FCLOSE (include/tst_safe_stdio.h), or document why
the raw call with continue-on-failure is intentional.

[...]

> +	len = readlink(driver_path, driver_link, sizeof(driver_link) - 1);

Use SAFE_READLINK (include/tst_safe_macros.h).

Also missing:
- testcases/kernel/device-drivers/nvme/.gitignore entry for nvme01
- runtest/ entry for nvme01 (e.g. in runtest/drivers)

---
Note:

Our agent completed the review of the patch. The full review can be
found at: (not available)

The agent can sometimes produce false positives although often its
findings are genuine. If you find issues with the review, please
comment this email or ignore the suggestions.

Regards,
LTP AI Reviewer

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp