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
Instead of each individual test removing this file, just do it in the common helper. Signed-off-by: Logan Gunthorpe <logang@deltatee.com> Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com> --- 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
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 <logang@deltatee.com> --- 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
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 <logang@deltatee.com> --- 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
This ensures we find the correct nvme loop device if others exist on a given system (which is generally not expected on test systems). Additionally, this will be required in the upcomming test nvme/037 which will have controllers racing with ones being destroyed. Signed-off-by: Logan Gunthorpe <logang@deltatee.com> --- tests/nvme/004 | 2 +- tests/nvme/005 | 2 +- tests/nvme/008 | 2 +- tests/nvme/009 | 2 +- tests/nvme/010 | 2 +- tests/nvme/011 | 2 +- tests/nvme/012 | 2 +- tests/nvme/013 | 2 +- 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/rc | 7 ++++--- 23 files changed, 26 insertions(+), 25 deletions(-) diff --git a/tests/nvme/004 b/tests/nvme/004 index dfca79aab20c..4b0b7ae50a5e 100755 --- a/tests/nvme/004 +++ b/tests/nvme/004 @@ -37,7 +37,7 @@ test() { _nvme_connect_subsys "${nvme_trtype}" blktests-subsystem-1 local nvmedev - nvmedev="$(_find_nvme_dev)" + nvmedev=$(_find_nvme_dev "blktests-subsystem-1") cat "/sys/block/${nvmedev}n1/uuid" cat "/sys/block/${nvmedev}n1/wwid" diff --git a/tests/nvme/005 b/tests/nvme/005 index 0d5801868bc0..9f3e388dc695 100755 --- a/tests/nvme/005 +++ b/tests/nvme/005 @@ -37,7 +37,7 @@ test() { _nvme_connect_subsys "${nvme_trtype}" blktests-subsystem-1 local nvmedev - nvmedev="$(_find_nvme_dev)" + nvmedev=$(_find_nvme_dev "blktests-subsystem-1") udevadm settle diff --git a/tests/nvme/008 b/tests/nvme/008 index 8616617ad398..219fe9b0ca6a 100755 --- a/tests/nvme/008 +++ b/tests/nvme/008 @@ -37,7 +37,7 @@ test() { _nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" - nvmedev="$(_find_nvme_dev)" + nvmedev=$(_find_nvme_dev "${subsys_name}") cat "/sys/block/${nvmedev}n1/uuid" cat "/sys/block/${nvmedev}n1/wwid" diff --git a/tests/nvme/009 b/tests/nvme/009 index e91d79065cb1..2814c79164ee 100755 --- a/tests/nvme/009 +++ b/tests/nvme/009 @@ -33,7 +33,7 @@ test() { _nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" - nvmedev="$(_find_nvme_dev)" + nvmedev=$(_find_nvme_dev "${subsys_name}") cat "/sys/block/${nvmedev}n1/uuid" cat "/sys/block/${nvmedev}n1/wwid" diff --git a/tests/nvme/010 b/tests/nvme/010 index 0188e842213e..150a4e540f3e 100755 --- a/tests/nvme/010 +++ b/tests/nvme/010 @@ -37,7 +37,7 @@ test() { _nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" - nvmedev="$(_find_nvme_dev)" + nvmedev=$(_find_nvme_dev "${subsys_name}") cat "/sys/block/${nvmedev}n1/uuid" cat "/sys/block/${nvmedev}n1/wwid" diff --git a/tests/nvme/011 b/tests/nvme/011 index 543dbe840874..4bfe9af084e4 100755 --- a/tests/nvme/011 +++ b/tests/nvme/011 @@ -35,7 +35,7 @@ test() { _nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" - nvmedev="$(_find_nvme_dev)" + nvmedev=$(_find_nvme_dev "${subsys_name}") cat "/sys/block/${nvmedev}n1/uuid" cat "/sys/block/${nvmedev}n1/wwid" diff --git a/tests/nvme/012 b/tests/nvme/012 index a13cd08ce6bf..c4e75b09796a 100755 --- a/tests/nvme/012 +++ b/tests/nvme/012 @@ -40,7 +40,7 @@ test() { _nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" - nvmedev="$(_find_nvme_dev)" + nvmedev=$(_find_nvme_dev "${subsys_name}") cat "/sys/block/${nvmedev}n1/uuid" cat "/sys/block/${nvmedev}n1/wwid" diff --git a/tests/nvme/013 b/tests/nvme/013 index 1ac725ea83f2..265b6968fd34 100755 --- a/tests/nvme/013 +++ b/tests/nvme/013 @@ -37,7 +37,7 @@ test() { _nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" - nvmedev="$(_find_nvme_dev)" + nvmedev=$(_find_nvme_dev "${subsys_name}") cat "/sys/block/${nvmedev}n1/uuid" cat "/sys/block/${nvmedev}n1/wwid" diff --git a/tests/nvme/014 b/tests/nvme/014 index e3c70364e332..48f8caaec0b3 100755 --- a/tests/nvme/014 +++ b/tests/nvme/014 @@ -37,7 +37,7 @@ test() { _nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" - nvmedev="$(_find_nvme_dev)" + nvmedev=$(_find_nvme_dev "${subsys_name}") cat "/sys/block/${nvmedev}n1/uuid" cat "/sys/block/${nvmedev}n1/wwid" diff --git a/tests/nvme/015 b/tests/nvme/015 index 46fa4f605749..e33cfde5d72e 100755 --- a/tests/nvme/015 +++ b/tests/nvme/015 @@ -34,7 +34,7 @@ test() { _nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" - nvmedev="$(_find_nvme_dev)" + nvmedev=$(_find_nvme_dev "${subsys_name}") cat "/sys/block/${nvmedev}n1/uuid" cat "/sys/block/${nvmedev}n1/wwid" diff --git a/tests/nvme/018 b/tests/nvme/018 index 6d7934d09d99..7f407da2ce19 100755 --- a/tests/nvme/018 +++ b/tests/nvme/018 @@ -35,7 +35,7 @@ test() { _nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" - nvmedev="$(_find_nvme_dev)" + nvmedev=$(_find_nvme_dev "${subsys_name}") cat "/sys/block/${nvmedev}n1/uuid" cat "/sys/block/${nvmedev}n1/wwid" diff --git a/tests/nvme/019 b/tests/nvme/019 index 486b5acff713..8259e2e0c157 100755 --- a/tests/nvme/019 +++ b/tests/nvme/019 @@ -39,7 +39,7 @@ test() { _nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" - nvmedev="$(_find_nvme_dev)" + nvmedev=$(_find_nvme_dev "${subsys_name}") cat "/sys/block/${nvmedev}n1/uuid" cat "/sys/block/${nvmedev}n1/wwid" diff --git a/tests/nvme/020 b/tests/nvme/020 index c8053f440e2e..16fdfcc94918 100755 --- a/tests/nvme/020 +++ b/tests/nvme/020 @@ -35,7 +35,7 @@ test() { _nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" - nvmedev="$(_find_nvme_dev)" + nvmedev=$(_find_nvme_dev "${subsys_name}") cat "/sys/block/${nvmedev}n1/uuid" cat "/sys/block/${nvmedev}n1/wwid" diff --git a/tests/nvme/021 b/tests/nvme/021 index f543a1d8fd92..fb77f9cbd99f 100755 --- a/tests/nvme/021 +++ b/tests/nvme/021 @@ -34,7 +34,7 @@ test() { _nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" - nvmedev="$(_find_nvme_dev)" + nvmedev=$(_find_nvme_dev "${subsys_name}") cat "/sys/block/${nvmedev}n1/uuid" cat "/sys/block/${nvmedev}n1/wwid" diff --git a/tests/nvme/022 b/tests/nvme/022 index e824ed31f6f0..62c4690e35fe 100755 --- a/tests/nvme/022 +++ b/tests/nvme/022 @@ -34,7 +34,7 @@ test() { _nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" - nvmedev="$(_find_nvme_dev)" + nvmedev=$(_find_nvme_dev "${subsys_name}") cat "/sys/block/${nvmedev}n1/uuid" cat "/sys/block/${nvmedev}n1/wwid" diff --git a/tests/nvme/023 b/tests/nvme/023 index bdef3dc8abca..bce21b56c9f1 100755 --- a/tests/nvme/023 +++ b/tests/nvme/023 @@ -37,7 +37,7 @@ test() { _nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" - nvmedev="$(_find_nvme_dev)" + nvmedev=$(_find_nvme_dev "${subsys_name}") cat "/sys/block/${nvmedev}n1/uuid" cat "/sys/block/${nvmedev}n1/wwid" diff --git a/tests/nvme/024 b/tests/nvme/024 index 78f779e8a08a..ffec36cf3333 100755 --- a/tests/nvme/024 +++ b/tests/nvme/024 @@ -34,7 +34,7 @@ test() { _nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" - nvmedev="$(_find_nvme_dev)" + nvmedev=$(_find_nvme_dev "${subsys_name}") cat "/sys/block/${nvmedev}n1/uuid" cat "/sys/block/${nvmedev}n1/wwid" diff --git a/tests/nvme/025 b/tests/nvme/025 index 223430965d7e..3d3f01bc45fd 100755 --- a/tests/nvme/025 +++ b/tests/nvme/025 @@ -34,7 +34,7 @@ test() { _nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" - nvmedev="$(_find_nvme_dev)" + nvmedev=$(_find_nvme_dev "${subsys_name}") cat "/sys/block/${nvmedev}n1/uuid" cat "/sys/block/${nvmedev}n1/wwid" diff --git a/tests/nvme/026 b/tests/nvme/026 index 7f82284d9c57..2f5607793cd3 100755 --- a/tests/nvme/026 +++ b/tests/nvme/026 @@ -34,7 +34,7 @@ test() { _nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" - nvmedev="$(_find_nvme_dev)" + nvmedev=$(_find_nvme_dev "${subsys_name}") cat "/sys/block/${nvmedev}n1/uuid" cat "/sys/block/${nvmedev}n1/wwid" diff --git a/tests/nvme/027 b/tests/nvme/027 index da96e6c5008d..53f06646a3d0 100755 --- a/tests/nvme/027 +++ b/tests/nvme/027 @@ -34,7 +34,7 @@ test() { _nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" - nvmedev="$(_find_nvme_dev)" + nvmedev=$(_find_nvme_dev "${subsys_name}") cat "/sys/block/${nvmedev}n1/uuid" cat "/sys/block/${nvmedev}n1/wwid" diff --git a/tests/nvme/028 b/tests/nvme/028 index f826b67623f1..3d9084f18636 100755 --- a/tests/nvme/028 +++ b/tests/nvme/028 @@ -34,7 +34,7 @@ test() { _nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" - nvmedev="$(_find_nvme_dev)" + nvmedev=$(_find_nvme_dev "${subsys_name}") cat "/sys/block/${nvmedev}n1/uuid" cat "/sys/block/${nvmedev}n1/wwid" diff --git a/tests/nvme/029 b/tests/nvme/029 index 5bed9b8e70ae..960e5f5a63bf 100755 --- a/tests/nvme/029 +++ b/tests/nvme/029 @@ -70,7 +70,7 @@ test() { _nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" - nvmedev="$(_find_nvme_dev)" + nvmedev=$(_find_nvme_dev "${subsys_name}") cat "/sys/block/${nvmedev}n1/uuid" cat "/sys/block/${nvmedev}n1/wwid" diff --git a/tests/nvme/rc b/tests/nvme/rc index 4c5b2e8edf0d..dfa57a299625 100644 --- a/tests/nvme/rc +++ b/tests/nvme/rc @@ -273,12 +273,13 @@ _remove_nvmet_subsystem_from_port() { } _find_nvme_dev() { + local subsys=$1 + local subsysnqn local dev - local transport for dev in /sys/class/nvme/nvme*; do dev="$(basename "$dev")" - transport="$(cat "/sys/class/nvme/${dev}/transport")" - if [[ "$transport" == "${nvme_trtype}" ]]; then + subsysnqn="$(cat "/sys/class/nvme/${dev}/subsysnqn")" + if [[ "$subsysnqn" == "$subsys" ]]; then echo "$dev" for ((i = 0; i < 10; i++)); do if [[ -e /sys/block/$dev/uuid && -- 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 <logang@deltatee.com> --- 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 <logang@deltatee.com> --- 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/010 and nvme/011 but for a passthru controller Signed-off-by: Logan Gunthorpe <logang@deltatee.com> --- 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
This is a similar test as nvme/012 and nvme/013, except with a passthru controller. Signed-off-by: Logan Gunthorpe <logang@deltatee.com> --- 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
Similar to test 022 but for passthru controllers. Signed-off-by: Logan Gunthorpe <logang@deltatee.com> --- 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
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 <logang@deltatee.com> --- 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
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 <logang@deltatee.com> --- 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
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 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 <logang@deltatee.com> > --- > 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 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/92478e6f-622a-a1ae-6189-4009f9a307bc@deltatee.com/
On 9/30/20 11:54, Logan Gunthorpe wrote:
> This ensures we find the correct nvme loop device if others exist on a
> given system (which is generally not expected on test systems).
>
> Additionally, this will be required in the upcomming test nvme/037 which
> will have controllers racing with ones being destroyed.
>
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
If I create a passthru testcase with :-
1. Create nvme-loop based nvme ctrl backed by null_blk say /dev/nvme1
2. Create a nvme-loop based passthru ctrl backed by /dev/nvme1 say nvme2.
With this patch or this series will I be able to write the testcase ?
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 <logang@deltatee.com> >> --- >> 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 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 <logang@deltatee.com> > --- > 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 5:55 p.m., Chaitanya Kulkarni wrote:
> On 9/30/20 11:54, Logan Gunthorpe wrote:
>> This ensures we find the correct nvme loop device if others exist on a
>> given system (which is generally not expected on test systems).
>>
>> Additionally, this will be required in the upcomming test nvme/037 which
>> will have controllers racing with ones being destroyed.
>>
>> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
>
> If I create a passthru testcase with :-
>
> 1. Create nvme-loop based nvme ctrl backed by null_blk say /dev/nvme1
>
> 2. Create a nvme-loop based passthru ctrl backed by /dev/nvme1 say nvme2.
>
>
> With this patch or this series will I be able to write the testcase ?
This patch helps with that but other helpers introduced in this series
would require minor changes.
As far as I can see, you'd only have to adjust _create_nvmet_passthru()
to take an optional argument because, presently, it always uses
$_test_dev_nvme_ctrl for the backing device.
This can easily be done if and when someone writes such a test.
However, I'm not even sure right now if that test would pass in the
kernel as is -- it seems like an odd thing to do.
Logan
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 <logang@deltatee.com> >> --- >> 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 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/92478e6f-622a-a1ae-6189-4009f9a307bc@deltatee.com/
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
On 2020-10-06 6:24 p.m., Chaitanya Kulkarni wrote: > On 10/6/20 17:10, Logan Gunthorpe wrote: >>> With this patch or this series will I be able to write the testcase ? >> This patch helps with that but other helpers introduced in this series >> would require minor changes. >> >> As far as I can see, you'd only have to adjust _create_nvmet_passthru() >> to take an optional argument because, presently, it always uses >> $_test_dev_nvme_ctrl for the backing device. >> >> This can easily be done if and when someone writes such a test. >> >> However, I'm not even sure right now if that test would pass in the >> kernel as is -- it seems like an odd thing to do. >> >> Logan >> > This test should pass if I remember the code correctly where we don't > > have any PCIe specific checks for the passthru controller and it is an Yes, there's no explicit restrictions, but that doesn't mean there are no bugs with that particular stack. > important to support this scenario in order to write device independent > > testcases as rest of the testcases are. Ok, feel free to write a test for this. It's not important to me. Logan