Hi,
This series adds blktests for the nvmet passthru feature that was merged
for 5.9. It's been reconciled with Sagi's blktest series that Omar
recently merged.
This series is based off of the current blktests master and a git repo is
available for this here:
https://github.com/Eideticom/blktests nvmet_passthru_v2
Thanks,
Logan
--
Changes in v2:
- Rebased on latest blktests master and changed to use the common
helpers Sagi introduced in his series
- Collected Chaitanya's reviewed-by tag
--
Logan Gunthorpe (11):
common/fio: Remove state file in common helper
common/xfs: Create common helper to check for XFS support
common/xfs: Create common helper to verify block device with xfs
nvme: Search for specific subsysnqn in _find_nvme_loop_dev
nvme: Add common helpers for passthru tests
nvme/033: Simple test to create and connect to a passthru target
nvme/034: Add test for passthru data verification
nvme/035: Add test to verify passthru controller with a filesystem
nvme/036: Add test for testing reset command on nvme-passthru
nvme/037: Add test which loops passthru connect and disconnect
nvme/038: Test removal of un-enabled subsystem and ports
common/fio | 1 +
common/rc | 8 +++++
common/xfs | 33 ++++++++++++++++++
tests/nvme/004 | 2 +-
tests/nvme/005 | 2 +-
tests/nvme/008 | 2 +-
tests/nvme/009 | 2 +-
tests/nvme/010 | 3 +-
tests/nvme/011 | 3 +-
tests/nvme/012 | 23 ++++---------
tests/nvme/013 | 21 +++---------
tests/nvme/014 | 2 +-
tests/nvme/015 | 2 +-
tests/nvme/018 | 2 +-
tests/nvme/019 | 2 +-
tests/nvme/020 | 2 +-
tests/nvme/021 | 2 +-
tests/nvme/022 | 2 +-
tests/nvme/023 | 2 +-
tests/nvme/024 | 2 +-
tests/nvme/025 | 2 +-
tests/nvme/026 | 2 +-
tests/nvme/027 | 2 +-
tests/nvme/028 | 2 +-
tests/nvme/029 | 2 +-
tests/nvme/033 | 67 +++++++++++++++++++++++++++++++++++++
tests/nvme/033.out | 7 ++++
tests/nvme/034 | 35 +++++++++++++++++++
tests/nvme/034.out | 3 ++
tests/nvme/035 | 37 +++++++++++++++++++++
tests/nvme/035.out | 3 ++
tests/nvme/036 | 37 +++++++++++++++++++++
tests/nvme/036.out | 3 ++
tests/nvme/037 | 35 +++++++++++++++++++
tests/nvme/037.out | 2 ++
tests/nvme/038 | 36 ++++++++++++++++++++
tests/nvme/038.out | 2 ++
tests/nvme/rc | 83 ++++++++++++++++++++++++++++++++++++++++++++--
38 files changed, 420 insertions(+), 58 deletions(-)
create mode 100644 common/xfs
create mode 100755 tests/nvme/033
create mode 100644 tests/nvme/033.out
create mode 100755 tests/nvme/034
create mode 100644 tests/nvme/034.out
create mode 100755 tests/nvme/035
create mode 100644 tests/nvme/035.out
create mode 100755 tests/nvme/036
create mode 100644 tests/nvme/036.out
create mode 100755 tests/nvme/037
create mode 100644 tests/nvme/037.out
create mode 100755 tests/nvme/038
create mode 100644 tests/nvme/038.out
base-commit: 20445c5eb6456addca9131ec6917d2a2d7414e04
--
2.20.1
Add some simple helpers to setup a passthru target that passes through
to a nvme test device.
Signed-off-by: Logan Gunthorpe <[email protected]>
---
tests/nvme/rc | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 76 insertions(+)
diff --git a/tests/nvme/rc b/tests/nvme/rc
index dfa57a299625..1ea23308a3f7 100644
--- a/tests/nvme/rc
+++ b/tests/nvme/rc
@@ -73,6 +73,17 @@ _require_nvme_trtype_is_fabrics() {
return 0
}
+_test_dev_nvme_ctrl() {
+ local dev
+
+ dev=$(cat "${TEST_DEV_SYSFS}/device/dev")
+ echo "/dev/char/${dev}"
+}
+
+_test_dev_nvme_nsid() {
+ cat "${TEST_DEV_SYSFS}/nsid"
+}
+
_cleanup_nvmet() {
local dev
local port
@@ -257,6 +268,27 @@ _remove_nvmet_subsystem() {
rmdir "${subsys_path}"
}
+_create_nvmet_passthru() {
+ local nvmet_subsystem="$1"
+ local subsys_path="${NVMET_CFS}/subsystems/${nvmet_subsystem}"
+ local passthru_path="${subsys_path}/passthru"
+
+ mkdir -p "${subsys_path}"
+ printf 1 > "${subsys_path}/attr_allow_any_host"
+
+ printf "%s" "$(_test_dev_nvme_ctrl)" > "${passthru_path}/device_path"
+ printf 1 > "${passthru_path}/enable"
+}
+
+_remove_nvmet_passhtru() {
+ local nvmet_subsystem="$1"
+ local subsys_path="${NVMET_CFS}/subsystems/${nvmet_subsystem}"
+ local passthru_path="${subsys_path}/passthru"
+
+ printf 0 > "${passthru_path}/enable"
+ rmdir "${subsys_path}"
+}
+
_add_nvmet_subsys_to_port() {
local port="$1"
local nvmet_subsystem="$2"
@@ -292,6 +324,50 @@ _find_nvme_dev() {
done
}
+_find_nvme_passthru_loop_dev() {
+ local subsys=$1
+ local nsid
+ local dev
+
+ dev=$(_find_nvme_dev "${subsys}")
+ nsid=$(_test_dev_nvme_nsid)
+ echo "/dev/${dev}n${nsid}"
+}
+
+_nvmet_passthru_target_setup() {
+ local subsys_name=$1
+
+ _create_nvmet_passthru "${subsys_name}"
+ port="$(_create_nvmet_port "loop")"
+ _add_nvmet_subsys_to_port "${port}" "${subsys_name}"
+
+ echo "$port"
+}
+
+_nvmet_passthru_target_connect() {
+ local trtype=$1
+ local subsys_name=$2
+
+ _nvme_connect_subsys "${trtype}" "${subsys_name}"
+ nsdev=$(_find_nvme_passthru_loop_dev "${subsys_name}")
+
+ # The following tests can race with the creation
+ # of the device so ensure the block device exists
+ # before continuing
+ while [ ! -b "${nsdev}" ]; do sleep 1; done
+
+ echo "${nsdev}"
+}
+
+_nvmet_passthru_target_cleanup() {
+ local port=$1
+ local subsys_name=$2
+
+ _remove_nvmet_subsystem_from_port "${port}" "${subsys_name}"
+ _remove_nvmet_port "${port}"
+ _remove_nvmet_passhtru "${subsys_name}"
+}
+
_filter_discovery() {
sed -n -r -e "s/Generation counter [0-9]+/Generation counter X/" \
-e '/Discovery Log Number|Log Entry|trtype|subnqn/p'
--
2.20.1
This tests creates and connects to a passthru controller backed
by a test NVMe namespace. It then verifies that some common fields
in id-ctrl and id-ns are the same in the target and the orginial
device.
Signed-off-by: Logan Gunthorpe <[email protected]>
---
tests/nvme/033 | 67 ++++++++++++++++++++++++++++++++++++++++++++++
tests/nvme/033.out | 7 +++++
2 files changed, 74 insertions(+)
create mode 100755 tests/nvme/033
create mode 100644 tests/nvme/033.out
diff --git a/tests/nvme/033 b/tests/nvme/033
new file mode 100755
index 000000000000..c6a3f7feb50e
--- /dev/null
+++ b/tests/nvme/033
@@ -0,0 +1,67 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2019 Logan Gunthorpe
+# Copyright (C) 2019 Eideticom Communications Inc.
+
+. tests/nvme/rc
+
+DESCRIPTION="create and connect to an NVMeOF target with a passthru controller"
+QUICK=1
+
+requires() {
+ _nvme_requires
+ _have_kernel_option NVME_TARGET_PASSTHRU
+}
+
+nvme_info() {
+ local ns=$1
+
+ nvme id-ctrl "${ns}" | grep -E '^(vid|sn|mn|fr) '
+ nvme id-ns "${ns}" | grep -E '^(nsze|ncap) '
+}
+
+compare_dev_info() {
+ local passthru_dev=$1
+ local testdev_info
+ local passthru_info
+
+ testdev_info=$(nvme_info "${TEST_DEV}")
+ passthru_info=$(nvme_info "${passthru_dev}")
+
+ cat >> "${FULL}" <<- EOF
+
+ Test Device ${TEST_DEV} Info:
+ $testdev_info
+
+ Passthru Device ${passthru_dev} Info:
+ $passthru_info
+
+ EOF
+
+ diff -u <(echo "${testdev_info}") <(echo "${passthru_info}")
+ if [[ "${testdev_info}" != "${passthru_info}" ]]; then
+ echo "ERROR: Device information does not match! (See ${FULL})"
+ fi
+}
+
+test_device() {
+ local subsys="blktests-subsystem-1"
+ local nsdev
+ local port
+
+ echo "Running ${TEST_NAME}"
+
+ _setup_nvmet
+ port=$(_nvmet_passthru_target_setup "${subsys}")
+
+ _nvme_discover "${nvme_trtype}" | _filter_discovery
+
+ nsdev=$(_nvmet_passthru_target_connect "${nvme_trtype}" "${subsys}")
+
+ compare_dev_info "${nsdev}"
+
+ _nvme_disconnect_subsys "${subsys}"
+ _nvmet_passthru_target_cleanup "${port}" "${subsys}"
+
+ echo "Test complete"
+}
diff --git a/tests/nvme/033.out b/tests/nvme/033.out
new file mode 100644
index 000000000000..6f45a1d5ec1d
--- /dev/null
+++ b/tests/nvme/033.out
@@ -0,0 +1,7 @@
+Running nvme/033
+Discovery Log Number of Records 1, Generation counter X
+=====Discovery Log Entry 0======
+trtype: loop
+subnqn: blktests-subsystem-1
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Test complete
--
2.20.1
Similar to test nvme/031 except for passthru controllers.
Note: it's normal to get I/O errors in this test as when the controller
disconnects it races with the partition table read.
Signed-off-by: Logan Gunthorpe <[email protected]>
---
tests/nvme/037 | 35 +++++++++++++++++++++++++++++++++++
tests/nvme/037.out | 2 ++
2 files changed, 37 insertions(+)
create mode 100755 tests/nvme/037
create mode 100644 tests/nvme/037.out
diff --git a/tests/nvme/037 b/tests/nvme/037
new file mode 100755
index 000000000000..fc6c21343652
--- /dev/null
+++ b/tests/nvme/037
@@ -0,0 +1,35 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2019 Logan Gunthorpe
+# Copyright (C) 2019 Eideticom Communications Inc.
+
+. tests/nvme/rc
+
+DESCRIPTION="test deletion of NVMeOF passthru controllers immediately after setup"
+
+requires() {
+ _nvme_requires
+ _have_kernel_option NVME_TARGET_PASSTHRU
+}
+
+test_device() {
+ local subsys="blktests-subsystem-"
+ local iterations=10
+ local ctrldev
+ local port
+
+ echo "Running ${TEST_NAME}"
+
+ _setup_nvmet
+
+ for ((i = 0; i < iterations; i++)); do
+ port=$(_nvmet_passthru_target_setup "${subsys}${i}")
+ nsdev=$(_nvmet_passthru_target_connect "${nvme_trtype}" \
+ "${subsys}${i}")
+
+ _nvme_disconnect_subsys "${subsys}${i}" >>"${FULL}" 2>&1
+ _nvmet_passthru_target_cleanup "${port}" "${subsys}${i}"
+ done
+
+ echo "Test complete"
+}
diff --git a/tests/nvme/037.out b/tests/nvme/037.out
new file mode 100644
index 000000000000..eaf903d0520e
--- /dev/null
+++ b/tests/nvme/037.out
@@ -0,0 +1,2 @@
+Running nvme/037
+Test complete
--
2.20.1
This is a similar test as nvme/012 and nvme/013, except with a
passthru controller.
Signed-off-by: Logan Gunthorpe <[email protected]>
---
tests/nvme/035 | 37 +++++++++++++++++++++++++++++++++++++
tests/nvme/035.out | 3 +++
2 files changed, 40 insertions(+)
create mode 100755 tests/nvme/035
create mode 100644 tests/nvme/035.out
diff --git a/tests/nvme/035 b/tests/nvme/035
new file mode 100755
index 000000000000..ee78a7586f35
--- /dev/null
+++ b/tests/nvme/035
@@ -0,0 +1,37 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2019 Logan Gunthorpe
+# Copyright (C) 2019 Eideticom Communications Inc.
+
+. tests/nvme/rc
+. common/xfs
+
+DESCRIPTION="run mkfs and data verification fio job on an NVMeOF passthru controller"
+TIMED=1
+
+requires() {
+ _nvme_requires
+ _have_kernel_option NVME_TARGET_PASSTHRU
+ _have_xfs
+ _have_fio
+}
+
+test_device() {
+ local subsys="blktests-subsystem-1"
+ local ctrldev
+ local nsdev
+ local port
+
+ echo "Running ${TEST_NAME}"
+
+ _setup_nvmet
+ port=$(_nvmet_passthru_target_setup "${subsys}")
+ nsdev=$(_nvmet_passthru_target_connect "${nvme_trtype}" "${subsys}")
+
+ _xfs_run_fio_verify_io "${nsdev}"
+
+ _nvme_disconnect_subsys "${subsys}"
+ _nvmet_passthru_target_cleanup "${port}" "${subsys}"
+
+ echo "Test complete"
+}
diff --git a/tests/nvme/035.out b/tests/nvme/035.out
new file mode 100644
index 000000000000..a6027138fbe4
--- /dev/null
+++ b/tests/nvme/035.out
@@ -0,0 +1,3 @@
+Running nvme/035
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Test complete
--
2.20.1
Two nvme tests create and mount XFS filesystems and check for mkfs.xfs.
They should also check for XFS support in the kernel so create a common
helper for this.
Signed-off-by: Logan Gunthorpe <[email protected]>
---
common/rc | 8 ++++++++
common/xfs | 11 +++++++++++
tests/nvme/012 | 6 ++++--
tests/nvme/013 | 4 +++-
4 files changed, 26 insertions(+), 3 deletions(-)
create mode 100644 common/xfs
diff --git a/common/rc b/common/rc
index cdc0150ea5ea..44cb218c2fac 100644
--- a/common/rc
+++ b/common/rc
@@ -181,6 +181,14 @@ _have_tracepoint() {
return 0
}
+_have_fs() {
+ modprobe "$1" >/dev/null 2>&1
+ if [[ ! -d "/sys/fs/$1" ]]; then
+ SKIP_REASON="kernel does not support filesystem $1"
+ return 1
+ fi
+}
+
_test_dev_is_rotational() {
[[ $(cat "${TEST_DEV_SYSFS}/queue/rotational") -ne 0 ]]
}
diff --git a/common/xfs b/common/xfs
new file mode 100644
index 000000000000..d1a603b8c7b5
--- /dev/null
+++ b/common/xfs
@@ -0,0 +1,11 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2017 Omar Sandoval
+#
+# fio helper functions.
+
+. common/shellcheck
+
+_have_xfs() {
+ _have_fs xfs && _have_program mkfs.xfs
+}
diff --git a/tests/nvme/012 b/tests/nvme/012
index 1dbd59804ed7..1d8d8e3cc271 100755
--- a/tests/nvme/012
+++ b/tests/nvme/012
@@ -5,14 +5,16 @@
# Test mkfs with data verification for block device backed ns.
. tests/nvme/rc
+. common/xfs
DESCRIPTION="run mkfs and data verification fio job on NVMeOF block device-backed ns"
TIMED=1
requires() {
_nvme_requires
- _have_program mkfs.xfs && _have_program fio && \
- _have_modules loop
+ _have_xfs
+ _have_program fio
+ _have_modules loop
_require_nvme_trtype_is_fabrics
}
diff --git a/tests/nvme/013 b/tests/nvme/013
index df7f23e69252..3819a2730d9b 100755
--- a/tests/nvme/013
+++ b/tests/nvme/013
@@ -5,13 +5,15 @@
# Test mkfs with data verification for file backed ns.
. tests/nvme/rc
+. common/xfs
DESCRIPTION="run mkfs and data verification fio job on NVMeOF file-backed ns"
TIMED=1
requires() {
_nvme_requires
- _have_program mkfs.xfs && _have_fio
+ _have_xfs
+ _have_fio
_require_nvme_trtype_is_fabrics
}
--
2.20.1
Similar to test nvme/010 and nvme/011 but for a passthru controller
Signed-off-by: Logan Gunthorpe <[email protected]>
---
tests/nvme/034 | 35 +++++++++++++++++++++++++++++++++++
tests/nvme/034.out | 3 +++
2 files changed, 38 insertions(+)
create mode 100755 tests/nvme/034
create mode 100644 tests/nvme/034.out
diff --git a/tests/nvme/034 b/tests/nvme/034
new file mode 100755
index 000000000000..f92e5e20865b
--- /dev/null
+++ b/tests/nvme/034
@@ -0,0 +1,35 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2019 Logan Gunthorpe
+# Copyright (C) 2019 Eideticom Communications Inc.
+
+. tests/nvme/rc
+
+DESCRIPTION="run data verification fio job on an NVMeOF passthru controller"
+TIMED=1
+
+requires() {
+ _nvme_requires
+ _have_kernel_option NVME_TARGET_PASSTHRU
+ _have_fio
+}
+
+test_device() {
+ local subsys="blktests-subsystem-1"
+ local ctrldev
+ local nsdev
+ local port
+
+ echo "Running ${TEST_NAME}"
+
+ _setup_nvmet
+ port=$(_nvmet_passthru_target_setup "${subsys}")
+ nsdev=$(_nvmet_passthru_target_connect "${nvme_trtype}" "${subsys}")
+
+ _run_fio_verify_io --size=950m --filename="${nsdev}"
+
+ _nvme_disconnect_subsys "${subsys}"
+ _nvmet_passthru_target_cleanup "${port}" "${subsys}"
+
+ echo "Test complete"
+}
diff --git a/tests/nvme/034.out b/tests/nvme/034.out
new file mode 100644
index 000000000000..0a7bd2f90dae
--- /dev/null
+++ b/tests/nvme/034.out
@@ -0,0 +1,3 @@
+Running nvme/034
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Test complete
--
2.20.1
Make a common helper from the code in tests nvme/012 and nvme/013
to run an fio verify on a XFS file system backed by the
specified block device.
While we are at it, all the output is redirected to $FULL instead of
/dev/null.
Signed-off-by: Logan Gunthorpe <[email protected]>
---
common/xfs | 22 ++++++++++++++++++++++
tests/nvme/012 | 14 +-------------
tests/nvme/013 | 14 +-------------
3 files changed, 24 insertions(+), 26 deletions(-)
diff --git a/common/xfs b/common/xfs
index d1a603b8c7b5..210c924cdd41 100644
--- a/common/xfs
+++ b/common/xfs
@@ -9,3 +9,25 @@
_have_xfs() {
_have_fs xfs && _have_program mkfs.xfs
}
+
+_xfs_mkfs_and_mount() {
+ local bdev=$1
+ local mount_dir=$2
+
+ mkdir -p "${mount_dir}"
+ umount "${mount_dir}"
+ mkfs.xfs -l size=32m -f "${bdev}"
+ mount "${bdev}" "${mount_dir}"
+}
+
+_xfs_run_fio_verify_io() {
+ local mount_dir="/mnt/blktests"
+ local bdev=$1
+
+ _xfs_mkfs_and_mount "${bdev}" "${mount_dir}" >> "${FULL}" 2>&1
+
+ _run_fio_verify_io --size=950m --directory="${mount_dir}/"
+
+ umount "${mount_dir}" >> "${FULL}" 2>&1
+ rm -fr "${mount_dir}"
+}
diff --git a/tests/nvme/012 b/tests/nvme/012
index 1d8d8e3cc271..a13cd08ce6bf 100755
--- a/tests/nvme/012
+++ b/tests/nvme/012
@@ -26,12 +26,9 @@ test() {
local port
local nvmedev
local loop_dev
- local mount_dir="/mnt/blktests"
local file_path="${TMPDIR}/img"
local subsys_name="blktests-subsystem-1"
- mkdir -p "${mount_dir}" > /dev/null 2>&1
-
truncate -s 1G "${file_path}"
loop_dev="$(losetup -f --show "${file_path}")"
@@ -47,15 +44,7 @@ test() {
cat "/sys/block/${nvmedev}n1/uuid"
cat "/sys/block/${nvmedev}n1/wwid"
- umount ${mount_dir} > /dev/null 2>&1
-
- mkfs.xfs -l size=32m -f /dev/"${nvmedev}n1" > /dev/null 2>&1
-
- mount /dev/"${nvmedev}n1" "${mount_dir}"
-
- _run_fio_verify_io --size=950m --directory="${mount_dir}/"
-
- umount "${mount_dir}" > /dev/null 2>&1
+ _xfs_run_fio_verify_io "/dev/${nvmedev}n1"
_nvme_disconnect_subsys "${subsys_name}"
@@ -66,7 +55,6 @@ test() {
losetup -d "${loop_dev}"
rm "${file_path}"
- rm -fr "${mount_dir}"
echo "Test complete"
}
diff --git a/tests/nvme/013 b/tests/nvme/013
index 3819a2730d9b..1ac725ea83f2 100755
--- a/tests/nvme/013
+++ b/tests/nvme/013
@@ -24,13 +24,10 @@ test() {
local port
local nvmedev
- local mount_dir="/mnt/blktests/"
local file_path="${TMPDIR}/img"
local subsys_name="blktests-subsystem-1"
- mkdir -p "${mount_dir}" > /dev/null 2>&1
-
truncate -s 1G "${file_path}"
_create_nvmet_subsystem "${subsys_name}" "${file_path}" \
@@ -44,15 +41,7 @@ test() {
cat "/sys/block/${nvmedev}n1/uuid"
cat "/sys/block/${nvmedev}n1/wwid"
- umount ${mount_dir} > /dev/null 2>&1
-
- mkfs.xfs -l size=32m -f /dev/"${nvmedev}n1" > /dev/null 2>&1
-
- mount /dev/"${nvmedev}n1" "${mount_dir}"
-
- _run_fio_verify_io --size=800m --directory="${mount_dir}/"
-
- umount "${mount_dir}" > /dev/null 2>&1
+ _xfs_run_fio_verify_io "/dev/${nvmedev}n1"
_nvme_disconnect_subsys "${subsys_name}"
@@ -61,7 +50,6 @@ test() {
_remove_nvmet_port "${port}"
rm "${file_path}"
- rm -fr "${mount_dir}"
echo "Test complete"
}
--
2.20.1
Test that we can remove a subsystem that has not been enabled by
passthru or any ns. Do the same for ports while we are at it.
This was an issue in the original passthru patches and is
not commonly tested. So this test will ensure we don't regress this.
Signed-off-by: Logan Gunthorpe <[email protected]>
---
tests/nvme/038 | 36 ++++++++++++++++++++++++++++++++++++
tests/nvme/038.out | 2 ++
2 files changed, 38 insertions(+)
create mode 100755 tests/nvme/038
create mode 100644 tests/nvme/038.out
diff --git a/tests/nvme/038 b/tests/nvme/038
new file mode 100755
index 000000000000..24f02d4ad4d1
--- /dev/null
+++ b/tests/nvme/038
@@ -0,0 +1,36 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2019 Logan Gunthorpe
+# Copyright (C) 2019 Eideticom Communications Inc.
+#
+# Test that we can remove a subsystem that has not been enabled by
+# passthru or any ns. Do the same for ports while we are at it.
+#
+# This was an issue in the original passthru patches and is
+# not commonly tested. So this test will ensure we don't regress this.
+#
+. tests/nvme/rc
+
+DESCRIPTION="test deletion of NVMeOF subsystem without enabling"
+QUICK=1
+
+requires() {
+ _nvme_requires
+}
+
+test() {
+ local subsys_path="${NVMET_CFS}/subsystems/blktests-subsystem-1"
+ local port
+
+ echo "Running ${TEST_NAME}"
+
+ _setup_nvmet
+
+ mkdir -p "${subsys_path}"
+ rmdir "${subsys_path}"
+
+ port=$(_create_nvmet_port loop)
+ _remove_nvmet_port "${port}"
+
+ echo "Test complete"
+}
diff --git a/tests/nvme/038.out b/tests/nvme/038.out
new file mode 100644
index 000000000000..06bc98022c33
--- /dev/null
+++ b/tests/nvme/038.out
@@ -0,0 +1,2 @@
+Running nvme/038
+Test complete
--
2.20.1
Instead of each individual test removing this file, just do it
in the common helper.
Signed-off-by: Logan Gunthorpe <[email protected]>
Reviewed-by: Chaitanya Kulkarni <[email protected]>
---
common/fio | 1 +
tests/nvme/010 | 1 -
tests/nvme/011 | 1 -
tests/nvme/012 | 1 -
tests/nvme/013 | 1 -
5 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/common/fio b/common/fio
index 8bfad4238dda..94c65c107a14 100644
--- a/common/fio
+++ b/common/fio
@@ -181,6 +181,7 @@ _run_fio_rand_io() {
_run_fio_verify_io() {
_run_fio --name=verify --rw=randwrite --direct=1 --ioengine=libaio --bs=4k \
--iodepth=16 --verify=crc32c "$@"
+ rm -f local*verify*state
}
_fio_perf_report() {
diff --git a/tests/nvme/010 b/tests/nvme/010
index 9d96d7803be3..0188e842213e 100755
--- a/tests/nvme/010
+++ b/tests/nvme/010
@@ -52,7 +52,6 @@ test() {
losetup -d "${loop_dev}"
rm "${file_path}"
- rm -f local*verify*state
echo "Test complete"
}
diff --git a/tests/nvme/011 b/tests/nvme/011
index 06dc568fb6ea..543dbe840874 100755
--- a/tests/nvme/011
+++ b/tests/nvme/011
@@ -48,7 +48,6 @@ test() {
_remove_nvmet_port "${port}"
rm "${file_path}"
- rm -f local-write-and-verify*state
echo "Test complete"
}
diff --git a/tests/nvme/012 b/tests/nvme/012
index 8110430e49d4..1dbd59804ed7 100755
--- a/tests/nvme/012
+++ b/tests/nvme/012
@@ -63,7 +63,6 @@ test() {
losetup -d "${loop_dev}"
- rm -f local*verify*state
rm "${file_path}"
rm -fr "${mount_dir}"
diff --git a/tests/nvme/013 b/tests/nvme/013
index 176b11b9ccb5..df7f23e69252 100755
--- a/tests/nvme/013
+++ b/tests/nvme/013
@@ -58,7 +58,6 @@ test() {
_remove_nvmet_subsystem "${subsys_name}"
_remove_nvmet_port "${port}"
- rm -f local*verify*state
rm "${file_path}"
rm -fr "${mount_dir}"
--
2.20.1
Similar to test 022 but for passthru controllers.
Signed-off-by: Logan Gunthorpe <[email protected]>
---
tests/nvme/036 | 37 +++++++++++++++++++++++++++++++++++++
tests/nvme/036.out | 3 +++
2 files changed, 40 insertions(+)
create mode 100755 tests/nvme/036
create mode 100644 tests/nvme/036.out
diff --git a/tests/nvme/036 b/tests/nvme/036
new file mode 100755
index 000000000000..8218c6538dfd
--- /dev/null
+++ b/tests/nvme/036
@@ -0,0 +1,37 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2019 Logan Gunthorpe
+# Copyright (C) 2019 Eideticom Communications Inc.
+
+. tests/nvme/rc
+
+DESCRIPTION="test NVMe reset command on an NVMeOF target with a passthru controller"
+QUICK=1
+
+requires() {
+ _nvme_requires
+ _have_kernel_option NVME_TARGET_PASSTHRU
+}
+
+test_device() {
+ local subsys="blktests-subsystem-1"
+ local ctrldev
+ local port
+
+ echo "Running ${TEST_NAME}"
+
+ _setup_nvmet
+ port=$(_nvmet_passthru_target_setup "${subsys}")
+ nsdev=$(_nvmet_passthru_target_connect "${nvme_trtype}" "${subsys}")
+
+ ctrldev=$(_find_nvme_dev "${subsys}")
+
+ if ! nvme reset "/dev/${ctrldev}" >> "$FULL" 2>&1; then
+ echo "ERROR: reset failed"
+ fi
+
+ _nvme_disconnect_subsys "${subsys}"
+ _nvmet_passthru_target_cleanup "${port}" "${subsys}"
+
+ echo "Test complete"
+}
diff --git a/tests/nvme/036.out b/tests/nvme/036.out
new file mode 100644
index 000000000000..d294f8646b20
--- /dev/null
+++ b/tests/nvme/036.out
@@ -0,0 +1,3 @@
+Running nvme/036
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Test complete
--
2.20.1
On 9/30/20 11:54, Logan Gunthorpe wrote:
> Make a common helper from the code in tests nvme/012 and nvme/013
> to run an fio verify on a XFS file system backed by the
> specified block device.
>
> While we are at it, all the output is redirected to $FULL instead of
> /dev/null.
>
> Signed-off-by: Logan Gunthorpe <[email protected]>
> ---
> common/xfs | 22 ++++++++++++++++++++++
> tests/nvme/012 | 14 +-------------
> tests/nvme/013 | 14 +-------------
> 3 files changed, 24 insertions(+), 26 deletions(-)
The common namespace is getting cluttered. Can you please create
a subdirectory common/fs/xfs ?
>
> diff --git a/common/xfs b/common/xfs
> index d1a603b8c7b5..210c924cdd41 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -9,3 +9,25 @@
> _have_xfs() {
> _have_fs xfs && _have_program mkfs.xfs
> }
> +
> +_xfs_mkfs_and_mount() {
> + local bdev=$1
> + local mount_dir=$2
> +
> + mkdir -p "${mount_dir}"
> + umount "${mount_dir}"
> + mkfs.xfs -l size=32m -f "${bdev}"
> + mount "${bdev}" "${mount_dir}"
> +}
> +
> +_xfs_run_fio_verify_io() {
> + local mount_dir="/mnt/blktests"
The mount dir should be a parameter and not the hardcode value
to make it reusable.
> + local bdev=$1
> +
> + _xfs_mkfs_and_mount "${bdev}" "${mount_dir}" >> "${FULL}" 2>&1
> +
> + _run_fio_verify_io --size=950m --directory="${mount_dir}/"
> +
> + umount "${mount_dir}" >> "${FULL}" 2>&1
> + rm -fr "${mount_dir}"
> +}
> diff --git a/tests/nvme/012 b/tests/nvme/012
> index 1d8d8e3cc271..a13cd08ce6bf 100755
> --- a/tests/nvme/012
> +++ b/tests/nvme/012
> @@ -26,12 +26,9 @@ test() {
> local port
> local nvmedev
> local loop_dev
> - local mount_dir="/mnt/blktests"
> local file_path="${TMPDIR}/img"
> local subsys_name="blktests-subsystem-1"
>
> - mkdir -p "${mount_dir}" > /dev/null 2>&1
> -
> truncate -s 1G "${file_path}"
>
> loop_dev="$(losetup -f --show "${file_path}")"
> @@ -47,15 +44,7 @@ test() {
> cat "/sys/block/${nvmedev}n1/uuid"
> cat "/sys/block/${nvmedev}n1/wwid"
>
> - umount ${mount_dir} > /dev/null 2>&1
> -
> - mkfs.xfs -l size=32m -f /dev/"${nvmedev}n1" > /dev/null 2>&1
> -
> - mount /dev/"${nvmedev}n1" "${mount_dir}"
> -
> - _run_fio_verify_io --size=950m --directory="${mount_dir}/"
> -
> - umount "${mount_dir}" > /dev/null 2>&1
> + _xfs_run_fio_verify_io "/dev/${nvmedev}n1"
>
> _nvme_disconnect_subsys "${subsys_name}"
>
> @@ -66,7 +55,6 @@ test() {
> losetup -d "${loop_dev}"
>
> rm "${file_path}"
> - rm -fr "${mount_dir}"
>
> echo "Test complete"
> }
> diff --git a/tests/nvme/013 b/tests/nvme/013
> index 3819a2730d9b..1ac725ea83f2 100755
> --- a/tests/nvme/013
> +++ b/tests/nvme/013
> @@ -24,13 +24,10 @@ test() {
>
> local port
> local nvmedev
> - local mount_dir="/mnt/blktests/"
> local file_path="${TMPDIR}/img"
>
> local subsys_name="blktests-subsystem-1"
>
> - mkdir -p "${mount_dir}" > /dev/null 2>&1
> -
> truncate -s 1G "${file_path}"
>
> _create_nvmet_subsystem "${subsys_name}" "${file_path}" \
> @@ -44,15 +41,7 @@ test() {
> cat "/sys/block/${nvmedev}n1/uuid"
> cat "/sys/block/${nvmedev}n1/wwid"
>
> - umount ${mount_dir} > /dev/null 2>&1
> -
> - mkfs.xfs -l size=32m -f /dev/"${nvmedev}n1" > /dev/null 2>&1
> -
> - mount /dev/"${nvmedev}n1" "${mount_dir}"
> -
> - _run_fio_verify_io --size=800m --directory="${mount_dir}/"
> -
> - umount "${mount_dir}" > /dev/null 2>&1
> + _xfs_run_fio_verify_io "/dev/${nvmedev}n1"
>
> _nvme_disconnect_subsys "${subsys_name}"
>
> @@ -61,7 +50,6 @@ test() {
> _remove_nvmet_port "${port}"
>
> rm "${file_path}"
> - rm -fr "${mount_dir}"
>
> echo "Test complete"
> }
rest looks good
On 9/30/20 11:54, Logan Gunthorpe wrote:
>
> requires() {
> _nvme_requires
> - _have_program mkfs.xfs && _have_fio
> + _have_xfs
> + _have_fio
Can you make _have_xfs return true false ? so it can be used with && ?
On 2020-10-06 5:44 p.m., Chaitanya Kulkarni wrote:
> On 9/30/20 11:54, Logan Gunthorpe wrote:
>>
>> requires() {
>> _nvme_requires
>> - _have_program mkfs.xfs && _have_fio
>> + _have_xfs
>> + _have_fio
> Can you make _have_xfs return true false ? so it can be used with && ?
_have_xfs() does return true/false and can be used with && or in a
conditional.
Per [1], my opinion is that using && in the requires() function where
the return value is ignored is confusing so I prefer not to do it in new
code.
If we want to reconsider this we, should add a check to ensure the
return value of requires() matches the expectation of the global
variable it uses.
Logan
[1]
https://lore.kernel.org/linux-block/[email protected]/
On 9/30/20 12:01, Logan Gunthorpe wrote:
> Add some simple helpers to setup a passthru target that passes through
> to a nvme test device.
>
> Signed-off-by: Logan Gunthorpe <[email protected]>
> ---
> tests/nvme/rc | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 76 insertions(+)
>
> diff --git a/tests/nvme/rc b/tests/nvme/rc
> index dfa57a299625..1ea23308a3f7 100644
> --- a/tests/nvme/rc
> +++ b/tests/nvme/rc
> @@ -73,6 +73,17 @@ _require_nvme_trtype_is_fabrics() {
> return 0
> }
>
> +_test_dev_nvme_ctrl() {
> + local dev
> +
> + dev=$(cat "${TEST_DEV_SYSFS}/device/dev")
can you initialize dev this at the time of declaration ?
> + echo "/dev/char/${dev}"
> +}
> +
> +_test_dev_nvme_nsid() {
> + cat "${TEST_DEV_SYSFS}/nsid"
> +}
> +
> _cleanup_nvmet() {
> local dev
> local port
> @@ -257,6 +268,27 @@ _remove_nvmet_subsystem() {
> rmdir "${subsys_path}"
> }
>
> +_create_nvmet_passthru() {
> + local nvmet_subsystem="$1"
> + local subsys_path="${NVMET_CFS}/subsystems/${nvmet_subsystem}"
> + local passthru_path="${subsys_path}/passthru"
> +
> + mkdir -p "${subsys_path}"
> + printf 1 > "${subsys_path}/attr_allow_any_host"
> +
> + printf "%s" "$(_test_dev_nvme_ctrl)" > "${passthru_path}/device_path"
> + printf 1 > "${passthru_path}/enable"
can you please echo in general and printf only when it is needed ?
I know existing code is a bit inconsistent I'll send a clenup to make it
uniform.
> +}
> +
> +_remove_nvmet_passhtru() {
> + local nvmet_subsystem="$1"
> + local subsys_path="${NVMET_CFS}/subsystems/${nvmet_subsystem}"
> + local passthru_path="${subsys_path}/passthru"
> +
> + printf 0 > "${passthru_path}/enable"
> + rmdir "${subsys_path}"
> +}
> +
> _add_nvmet_subsys_to_port() {
> local port="$1"
> local nvmet_subsystem="$2"
> @@ -292,6 +324,50 @@ _find_nvme_dev() {
> done
> }
>
> +_find_nvme_passthru_loop_dev() {
> + local subsys=$1
> + local nsid
> + local dev
> +
> + dev=$(_find_nvme_dev "${subsys}")
> + nsid=$(_test_dev_nvme_nsid)
> + echo "/dev/${dev}n${nsid}"
> +}
> +
> +_nvmet_passthru_target_setup() {
> + local subsys_name=$1
> +
> + _create_nvmet_passthru "${subsys_name}"
> + port="$(_create_nvmet_port "loop")"
> + _add_nvmet_subsys_to_port "${port}" "${subsys_name}"
> +
> + echo "$port"
> +}
> +
> +_nvmet_passthru_target_connect() {
> + local trtype=$1
> + local subsys_name=$2
> +
> + _nvme_connect_subsys "${trtype}" "${subsys_name}"
> + nsdev=$(_find_nvme_passthru_loop_dev "${subsys_name}")
> +
> + # The following tests can race with the creation
> + # of the device so ensure the block device exists
> + # before continuing
> + while [ ! -b "${nsdev}" ]; do sleep 1; done
> +
> + echo "${nsdev}"
> +}
> +
> +_nvmet_passthru_target_cleanup() {
> + local port=$1
> + local subsys_name=$2
> +
> + _remove_nvmet_subsystem_from_port "${port}" "${subsys_name}"
> + _remove_nvmet_port "${port}"
> + _remove_nvmet_passhtru "${subsys_name}"
> +}
> +
> _filter_discovery() {
> sed -n -r -e "s/Generation counter [0-9]+/Generation counter X/" \
> -e '/Discovery Log Number|Log Entry|trtype|subnqn/p'
On 2020-10-06 6:02 p.m., Chaitanya Kulkarni wrote:
> On 9/30/20 12:01, Logan Gunthorpe wrote:
>> Add some simple helpers to setup a passthru target that passes through
>> to a nvme test device.
>>
>> Signed-off-by: Logan Gunthorpe <[email protected]>
>> ---
>> tests/nvme/rc | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 76 insertions(+)
>>
>> diff --git a/tests/nvme/rc b/tests/nvme/rc
>> index dfa57a299625..1ea23308a3f7 100644
>> --- a/tests/nvme/rc
>> +++ b/tests/nvme/rc
>> @@ -73,6 +73,17 @@ _require_nvme_trtype_is_fabrics() {
>> return 0
>> }
>>
>> +_test_dev_nvme_ctrl() {
>> + local dev
>> +
>> + dev=$(cat "${TEST_DEV_SYSFS}/device/dev")
> can you initialize dev this at the time of declaration ?
Yup, will fix.
>> + echo "/dev/char/${dev}"
>> +}
>> +
>> +_test_dev_nvme_nsid() {
>> + cat "${TEST_DEV_SYSFS}/nsid"
>> +}
>> +
>> _cleanup_nvmet() {
>> local dev
>> local port
>> @@ -257,6 +268,27 @@ _remove_nvmet_subsystem() {
>> rmdir "${subsys_path}"
>> }
>>
>> +_create_nvmet_passthru() {
>> + local nvmet_subsystem="$1"
>> + local subsys_path="${NVMET_CFS}/subsystems/${nvmet_subsystem}"
>> + local passthru_path="${subsys_path}/passthru"
>> +
>> + mkdir -p "${subsys_path}"
>> + printf 1 > "${subsys_path}/attr_allow_any_host"
>> +
>> + printf "%s" "$(_test_dev_nvme_ctrl)" > "${passthru_path}/device_path"
>> + printf 1 > "${passthru_path}/enable"
>
> can you please echo in general and printf only when it is needed ?>
> I know existing code is a bit inconsistent I'll send a clenup to make it
> uniform.
Yes, I agree. I will it fix in v3.
Thanks,
Logan
On 2020-10-06 5:50 p.m., Chaitanya Kulkarni wrote:
> On 9/30/20 11:54, Logan Gunthorpe wrote:
>> Make a common helper from the code in tests nvme/012 and nvme/013
>> to run an fio verify on a XFS file system backed by the
>> specified block device.
>>
>> While we are at it, all the output is redirected to $FULL instead of
>> /dev/null.
>>
>> Signed-off-by: Logan Gunthorpe <[email protected]>
>> ---
>> common/xfs | 22 ++++++++++++++++++++++
>> tests/nvme/012 | 14 +-------------
>> tests/nvme/013 | 14 +-------------
>> 3 files changed, 24 insertions(+), 26 deletions(-)
>
> The common namespace is getting cluttered. Can you please create
>
> a subdirectory common/fs/xfs ?
I disagree. The common directory only has 9 files. And given there
appear to be no other files to add to the fs directory I don't think now
is the time to create a directory. We can do so if and when a number of
other fs helpers show up and there's a reason to group them together.
>>
>> diff --git a/common/xfs b/common/xfs
>> index d1a603b8c7b5..210c924cdd41 100644
>> --- a/common/xfs
>> +++ b/common/xfs
>> @@ -9,3 +9,25 @@
>> _have_xfs() {
>> _have_fs xfs && _have_program mkfs.xfs
>> }
>> +
>> +_xfs_mkfs_and_mount() {
>> + local bdev=$1
>> + local mount_dir=$2
>> +
>> + mkdir -p "${mount_dir}"
>> + umount "${mount_dir}"
>> + mkfs.xfs -l size=32m -f "${bdev}"
>> + mount "${bdev}" "${mount_dir}"
>> +}
>> +
>> +_xfs_run_fio_verify_io() {
>> + local mount_dir="/mnt/blktests"
>
> The mount dir should be a parameter and not the hardcode value
> to make it reusable.
I also disagree here. It is already reusable and is used in a number of
places; none of those places require changing the mount directory. If
and when a use comes up that requires a different directory (not sure
what that would be), a parameter can be added. It is typically standard
practice in the Linux community to not add features that have no users
as it's confusing to people reading the code.
Logan
On 10/6/20 16:51, Logan Gunthorpe wrote:
> _have_xfs() does return true/false and can be used with && or in a
> conditional.
>
> Per [1], my opinion is that using && in the requires() function where
> the return value is ignored is confusing so I prefer not to do it in new
> code.
>
> If we want to reconsider this we, should add a check to ensure the
> return value of requires() matches the expectation of the global
> variable it uses.
>
> Logan
>
> [1]
> https://lore.kernel.org/linux-block/[email protected]/
Make sense to me, lets not change this, thanks for pointing that out.
On 2020-10-06 6:20 p.m., Chaitanya Kulkarni wrote:
> On 10/6/20 16:59, Logan Gunthorpe wrote:
>>> The mount dir should be a parameter and not the hardcode value
>>> to make it reusable.
>> I also disagree here. It is already reusable and is used in a number of
>> places; none of those places require changing the mount directory. If
>> and when a use comes up that requires a different directory (not sure
>> what that would be), a parameter can be added. It is typically standard
>> practice in the Linux community to not add features that have no users
>> as it's confusing to people reading the code.
>>
>> Logan
>>
> Well if you are making a helper it should be generic if you have a usecase,
"Generic" isn't a binary yes/no quality. Why add the mount option (that nobody is using)
and not a size option as well that nobody uses? For that matter, fio has a ton of options
we could expose. (think io-method, read/write pattern, etc, etc). The criteria we
decide upon which options get exposed as arguments is what the code that's actually
using it needs -- not what's available or what you imagine future use cases might be.
If there are no users in the code it should not be exposed. If a use case comes along,
an argument can easily be added when the new test is added to the code base.
> mounted on different mount points not just one which is important testcase,
>
> that will require a prep patch.
So? That's normal.
> Why can't we do that right now when we have a clear usecase ?
We don't have a clear use case that's being added to the code though... We
have an imagined use case that hasn't been written. Add the feature when you
add this use case.
Logan