On Jun 09, 2025 / 08:41, Keith Busch wrote:
...
> diff --git a/tests/nvme/064 b/tests/nvme/064
> new file mode 100755
> index 0000000..fd72d4a
> --- /dev/null
> +++ b/tests/nvme/064
...
> +test_device() {
> + echo "Running ${TEST_NAME}"
> +
> + if src/nvme-passthrough-meta "${TEST_DEV}"; then
I think the check logic above should be inverted:
if ! src/nvme-passthrough-meta "${TEST_DEV}"; then
Thanks for this v2. With the fix above, I was able to confirme that the test
case passes with v6.16-rc1 kernel. When I reverted the kernel commit below,
it failed. It looks working good as the fix confirmation.
43a67dd812c5 ("block: flip iter directions in blk_rq_integrity_map_user()")
To run the test case, I tried QEMU nvme emulation devices with some different
options. I found that the namespace should have format with metadata, and
extended LBA should be disabled. IOW, QEMU -drive option should have value
"pi=1,pil=1,ms=8" for the namespace.
I suggest to describe the device requirements in the test case comment. Also, I
suggest to check the requirements for the test case, and skip if the
requirements are not fulfilled. FYI, I prototyped such change as the patch
below. Please let me know what your think. If you are okay with it, I will
repost your patch together with my patch for common/rc and tests/nvme/rc as the
v3 series.
diff --git a/common/rc b/common/rc
index ff3f0a3..7155233 100644
--- a/common/rc
+++ b/common/rc
@@ -422,6 +422,14 @@ _test_dev_is_partition() {
[[ -n ${TEST_DEV_PART_SYSFS} ]]
}
+_test_dev_has_metadata() {
+ if (( ! $(<"${TEST_DEV_SYSFS}/metadata_bytes") )); then
+ SKIP_REASONS+=("$TEST_DEV does not have metadata")
+ return 1
+ fi
+ return 0
+}
+
_min_io() {
local path_or_dev=$1
local min_io
diff --git a/src/nvme-passthrough-meta.c b/src/nvme-passthrough-meta.c
index d19ee25..29980a2 100644
--- a/src/nvme-passthrough-meta.c
+++ b/src/nvme-passthrough-meta.c
@@ -139,8 +139,10 @@ int main(int argc, char **argv)
meta_size = lbaf.ms;
/* format not appropriate for this test */
- if (meta_size == 0)
- return 0;
+ if (meta_size == 0) {
+ fprintf(stderr, "Device format does not have metadata\n");
+ return -EINVAL;
+ }
blocks = BUFFER_SIZE / block_size;
meta_buffer_size = blocks * meta_size;
diff --git a/tests/nvme/064 b/tests/nvme/064
index fd72d4a..eb9ca50 100755
--- a/tests/nvme/064
+++ b/tests/nvme/064
@@ -2,7 +2,12 @@
# SPDX-License-Identifier: GPL-3.0+
# Copyright (C) 2025 Keith Busch <kbusch@xxxxxxxxxx>
#
-# Test out metadata through the passthrough interfaces
+# Test out metadata through the passthrough interfaces. This test confirms the
+# fix by the kernel commit 43a67dd812c5 ("block: flip iter directions in
+# blk_rq_integrity_map_user()"). This test requires TEST_DEV as a namespace
+# formatted with metadata, and extended LBA disabled. Such namespace can be
+# prepared with QEMU NVME emulation specifying -device option with
+# "pi=1,pil=1,ms=8" values.
. tests/nvme/rc
@@ -10,13 +15,18 @@ requires() {
_nvme_requires
}
+device_requires() {
+ _test_dev_has_metadata
+ _require_test_dev_extended_lba_off
+}
+
DESCRIPTION="exercise the nvme metadata usage with passthrough commands"
QUICK=1
test_device() {
echo "Running ${TEST_NAME}"
- if src/nvme-passthrough-meta "${TEST_DEV}"; then
+ if ! src/nvme-passthrough-meta "${TEST_DEV}"; then
echo "src/nvme-passthrough-meta failed"
fi
diff --git a/tests/nvme/rc b/tests/nvme/rc
index 9dbd1ce..f69ceb9 100644
--- a/tests/nvme/rc
+++ b/tests/nvme/rc
@@ -153,6 +153,21 @@ _require_test_dev_is_not_nvme_multipath() {
return 0
}
+_require_test_dev_extended_lba_off() {
+ local flbas
+
+ if ! flbas=$(nvme id-ns "$TEST_DEV" | grep flbas | \
+ sed --quiet 's/.*: \(.*\)/\1/p'); then
+ SKIP_REASONS+=("$TEST_DEV does not have namespace flbas field")
+ return 1
+ fi
+ if (( ${flbas} & 0x10 )); then
+ SKIP_REASONS+=("$TEST_DEV has NVME_NS_FLBAS_META_EXT on")
+ return 1
+ fi
+ return 0
+}
+
_require_nvme_test_img_size() {
local require_sz_mb
local nvme_img_size_mb
