linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH blktests v2 00/11] NVMe Target Passthru Block Tests
@ 2020-09-30 18:54 Logan Gunthorpe
  2020-09-30 18:54 ` [PATCH blktests v2 01/11] common/fio: Remove state file in common helper Logan Gunthorpe
                   ` (10 more replies)
  0 siblings, 11 replies; 23+ messages in thread
From: Logan Gunthorpe @ 2020-09-30 18:54 UTC (permalink / raw)
  To: linux-kernel, linux-nvme, linux-block, Omar Sandoval
  Cc: Sagi Grimberg, Chaitanya Kulkarni, Stephen Bates, Logan Gunthorpe

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH blktests v2 01/11] common/fio: Remove state file in common helper
  2020-09-30 18:54 [PATCH blktests v2 00/11] NVMe Target Passthru Block Tests Logan Gunthorpe
@ 2020-09-30 18:54 ` Logan Gunthorpe
  2020-09-30 18:54 ` [PATCH blktests v2 02/11] common/xfs: Create common helper to check for XFS support Logan Gunthorpe
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Logan Gunthorpe @ 2020-09-30 18:54 UTC (permalink / raw)
  To: linux-kernel, linux-nvme, linux-block, Omar Sandoval
  Cc: Sagi Grimberg, Chaitanya Kulkarni, Stephen Bates,
	Logan Gunthorpe, Chaitanya Kulkarni

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


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH blktests v2 02/11] common/xfs: Create common helper to check for XFS support
  2020-09-30 18:54 [PATCH blktests v2 00/11] NVMe Target Passthru Block Tests Logan Gunthorpe
  2020-09-30 18:54 ` [PATCH blktests v2 01/11] common/fio: Remove state file in common helper Logan Gunthorpe
@ 2020-09-30 18:54 ` Logan Gunthorpe
  2020-10-06 23:44   ` Chaitanya Kulkarni
  2020-09-30 18:54 ` [PATCH blktests v2 03/11] common/xfs: Create common helper to verify block device with xfs Logan Gunthorpe
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Logan Gunthorpe @ 2020-09-30 18:54 UTC (permalink / raw)
  To: linux-kernel, linux-nvme, linux-block, Omar Sandoval
  Cc: Sagi Grimberg, Chaitanya Kulkarni, Stephen Bates, Logan Gunthorpe

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


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH blktests v2 03/11] common/xfs: Create common helper to verify block device with xfs
  2020-09-30 18:54 [PATCH blktests v2 00/11] NVMe Target Passthru Block Tests Logan Gunthorpe
  2020-09-30 18:54 ` [PATCH blktests v2 01/11] common/fio: Remove state file in common helper Logan Gunthorpe
  2020-09-30 18:54 ` [PATCH blktests v2 02/11] common/xfs: Create common helper to check for XFS support Logan Gunthorpe
@ 2020-09-30 18:54 ` Logan Gunthorpe
  2020-10-06 23:50   ` Chaitanya Kulkarni
  2020-09-30 18:54 ` [PATCH blktests v2 04/11] nvme: Search for specific subsysnqn in _find_nvme_loop_dev Logan Gunthorpe
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Logan Gunthorpe @ 2020-09-30 18:54 UTC (permalink / raw)
  To: linux-kernel, linux-nvme, linux-block, Omar Sandoval
  Cc: Sagi Grimberg, Chaitanya Kulkarni, Stephen Bates, Logan Gunthorpe

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


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH blktests v2 04/11] nvme: Search for specific subsysnqn in _find_nvme_loop_dev
  2020-09-30 18:54 [PATCH blktests v2 00/11] NVMe Target Passthru Block Tests Logan Gunthorpe
                   ` (2 preceding siblings ...)
  2020-09-30 18:54 ` [PATCH blktests v2 03/11] common/xfs: Create common helper to verify block device with xfs Logan Gunthorpe
@ 2020-09-30 18:54 ` Logan Gunthorpe
  2020-10-06 23:55   ` Chaitanya Kulkarni
  2020-09-30 18:54 ` [PATCH blktests v2 05/11] nvme: Add common helpers for passthru tests Logan Gunthorpe
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Logan Gunthorpe @ 2020-09-30 18:54 UTC (permalink / raw)
  To: linux-kernel, linux-nvme, linux-block, Omar Sandoval
  Cc: Sagi Grimberg, Chaitanya Kulkarni, Stephen Bates, Logan Gunthorpe

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


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH blktests v2 05/11] nvme: Add common helpers for passthru tests
  2020-09-30 18:54 [PATCH blktests v2 00/11] NVMe Target Passthru Block Tests Logan Gunthorpe
                   ` (3 preceding siblings ...)
  2020-09-30 18:54 ` [PATCH blktests v2 04/11] nvme: Search for specific subsysnqn in _find_nvme_loop_dev Logan Gunthorpe
@ 2020-09-30 18:54 ` Logan Gunthorpe
  2020-10-07  0:02   ` Chaitanya Kulkarni
  2020-09-30 18:54 ` [PATCH blktests v2 06/11] nvme/033: Simple test to create and connect to a passthru target Logan Gunthorpe
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Logan Gunthorpe @ 2020-09-30 18:54 UTC (permalink / raw)
  To: linux-kernel, linux-nvme, linux-block, Omar Sandoval
  Cc: Sagi Grimberg, Chaitanya Kulkarni, Stephen Bates, Logan Gunthorpe

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


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH blktests v2 06/11] nvme/033: Simple test to create and connect to a passthru target
  2020-09-30 18:54 [PATCH blktests v2 00/11] NVMe Target Passthru Block Tests Logan Gunthorpe
                   ` (4 preceding siblings ...)
  2020-09-30 18:54 ` [PATCH blktests v2 05/11] nvme: Add common helpers for passthru tests Logan Gunthorpe
@ 2020-09-30 18:54 ` Logan Gunthorpe
  2020-09-30 18:54 ` [PATCH blktests v2 07/11] nvme/034: Add test for passthru data verification Logan Gunthorpe
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Logan Gunthorpe @ 2020-09-30 18:54 UTC (permalink / raw)
  To: linux-kernel, linux-nvme, linux-block, Omar Sandoval
  Cc: Sagi Grimberg, Chaitanya Kulkarni, Stephen Bates, Logan Gunthorpe

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


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH blktests v2 07/11] nvme/034: Add test for passthru data verification
  2020-09-30 18:54 [PATCH blktests v2 00/11] NVMe Target Passthru Block Tests Logan Gunthorpe
                   ` (5 preceding siblings ...)
  2020-09-30 18:54 ` [PATCH blktests v2 06/11] nvme/033: Simple test to create and connect to a passthru target Logan Gunthorpe
@ 2020-09-30 18:54 ` Logan Gunthorpe
  2020-09-30 18:54 ` [PATCH blktests v2 08/11] nvme/035: Add test to verify passthru controller with a filesystem Logan Gunthorpe
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Logan Gunthorpe @ 2020-09-30 18:54 UTC (permalink / raw)
  To: linux-kernel, linux-nvme, linux-block, Omar Sandoval
  Cc: Sagi Grimberg, Chaitanya Kulkarni, Stephen Bates, Logan Gunthorpe

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


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH blktests v2 08/11] nvme/035: Add test to verify passthru controller with a filesystem
  2020-09-30 18:54 [PATCH blktests v2 00/11] NVMe Target Passthru Block Tests Logan Gunthorpe
                   ` (6 preceding siblings ...)
  2020-09-30 18:54 ` [PATCH blktests v2 07/11] nvme/034: Add test for passthru data verification Logan Gunthorpe
@ 2020-09-30 18:54 ` Logan Gunthorpe
  2020-09-30 18:54 ` [PATCH blktests v2 09/11] nvme/036: Add test for testing reset command on nvme-passthru Logan Gunthorpe
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Logan Gunthorpe @ 2020-09-30 18:54 UTC (permalink / raw)
  To: linux-kernel, linux-nvme, linux-block, Omar Sandoval
  Cc: Sagi Grimberg, Chaitanya Kulkarni, Stephen Bates, Logan Gunthorpe

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


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH blktests v2 09/11] nvme/036: Add test for testing reset command on nvme-passthru
  2020-09-30 18:54 [PATCH blktests v2 00/11] NVMe Target Passthru Block Tests Logan Gunthorpe
                   ` (7 preceding siblings ...)
  2020-09-30 18:54 ` [PATCH blktests v2 08/11] nvme/035: Add test to verify passthru controller with a filesystem Logan Gunthorpe
@ 2020-09-30 18:54 ` Logan Gunthorpe
  2020-09-30 18:54 ` [PATCH blktests v2 10/11] nvme/037: Add test which loops passthru connect and disconnect Logan Gunthorpe
  2020-09-30 18:54 ` [PATCH blktests v2 11/11] nvme/038: Test removal of un-enabled subsystem and ports Logan Gunthorpe
  10 siblings, 0 replies; 23+ messages in thread
From: Logan Gunthorpe @ 2020-09-30 18:54 UTC (permalink / raw)
  To: linux-kernel, linux-nvme, linux-block, Omar Sandoval
  Cc: Sagi Grimberg, Chaitanya Kulkarni, Stephen Bates, Logan Gunthorpe

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


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH blktests v2 10/11] nvme/037: Add test which loops passthru connect and disconnect
  2020-09-30 18:54 [PATCH blktests v2 00/11] NVMe Target Passthru Block Tests Logan Gunthorpe
                   ` (8 preceding siblings ...)
  2020-09-30 18:54 ` [PATCH blktests v2 09/11] nvme/036: Add test for testing reset command on nvme-passthru Logan Gunthorpe
@ 2020-09-30 18:54 ` Logan Gunthorpe
  2020-09-30 18:54 ` [PATCH blktests v2 11/11] nvme/038: Test removal of un-enabled subsystem and ports Logan Gunthorpe
  10 siblings, 0 replies; 23+ messages in thread
From: Logan Gunthorpe @ 2020-09-30 18:54 UTC (permalink / raw)
  To: linux-kernel, linux-nvme, linux-block, Omar Sandoval
  Cc: Sagi Grimberg, Chaitanya Kulkarni, Stephen Bates, Logan Gunthorpe

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


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH blktests v2 11/11] nvme/038: Test removal of un-enabled subsystem and ports
  2020-09-30 18:54 [PATCH blktests v2 00/11] NVMe Target Passthru Block Tests Logan Gunthorpe
                   ` (9 preceding siblings ...)
  2020-09-30 18:54 ` [PATCH blktests v2 10/11] nvme/037: Add test which loops passthru connect and disconnect Logan Gunthorpe
@ 2020-09-30 18:54 ` Logan Gunthorpe
  10 siblings, 0 replies; 23+ messages in thread
From: Logan Gunthorpe @ 2020-09-30 18:54 UTC (permalink / raw)
  To: linux-kernel, linux-nvme, linux-block, Omar Sandoval
  Cc: Sagi Grimberg, Chaitanya Kulkarni, Stephen Bates, Logan Gunthorpe

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


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH blktests v2 02/11] common/xfs: Create common helper to check for XFS support
  2020-09-30 18:54 ` [PATCH blktests v2 02/11] common/xfs: Create common helper to check for XFS support Logan Gunthorpe
@ 2020-10-06 23:44   ` Chaitanya Kulkarni
  2020-10-06 23:51     ` Logan Gunthorpe
  0 siblings, 1 reply; 23+ messages in thread
From: Chaitanya Kulkarni @ 2020-10-06 23:44 UTC (permalink / raw)
  To: Logan Gunthorpe, linux-kernel, linux-nvme, linux-block, Omar Sandoval
  Cc: Sagi Grimberg, Stephen Bates

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 && ?

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH blktests v2 03/11] common/xfs: Create common helper to verify block device with xfs
  2020-09-30 18:54 ` [PATCH blktests v2 03/11] common/xfs: Create common helper to verify block device with xfs Logan Gunthorpe
@ 2020-10-06 23:50   ` Chaitanya Kulkarni
  2020-10-06 23:59     ` Logan Gunthorpe
  0 siblings, 1 reply; 23+ messages in thread
From: Chaitanya Kulkarni @ 2020-10-06 23:50 UTC (permalink / raw)
  To: Logan Gunthorpe, linux-kernel, linux-nvme, linux-block, Omar Sandoval
  Cc: Sagi Grimberg, Stephen Bates

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


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH blktests v2 02/11] common/xfs: Create common helper to check for XFS support
  2020-10-06 23:44   ` Chaitanya Kulkarni
@ 2020-10-06 23:51     ` Logan Gunthorpe
  2020-10-07  0:58       ` Chaitanya Kulkarni
  0 siblings, 1 reply; 23+ messages in thread
From: Logan Gunthorpe @ 2020-10-06 23:51 UTC (permalink / raw)
  To: Chaitanya Kulkarni, linux-kernel, linux-nvme, linux-block, Omar Sandoval
  Cc: Sagi Grimberg, Stephen Bates



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/

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH blktests v2 04/11] nvme: Search for specific subsysnqn in _find_nvme_loop_dev
  2020-09-30 18:54 ` [PATCH blktests v2 04/11] nvme: Search for specific subsysnqn in _find_nvme_loop_dev Logan Gunthorpe
@ 2020-10-06 23:55   ` Chaitanya Kulkarni
  2020-10-07  0:10     ` Logan Gunthorpe
  0 siblings, 1 reply; 23+ messages in thread
From: Chaitanya Kulkarni @ 2020-10-06 23:55 UTC (permalink / raw)
  To: Logan Gunthorpe, linux-kernel, linux-nvme, linux-block, Omar Sandoval
  Cc: Sagi Grimberg, Stephen Bates

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 ?


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH blktests v2 03/11] common/xfs: Create common helper to verify block device with xfs
  2020-10-06 23:50   ` Chaitanya Kulkarni
@ 2020-10-06 23:59     ` Logan Gunthorpe
       [not found]       ` <BYAPR04MB49652338A4FE3805F9394A88860A0@BYAPR04MB4965.namprd04.prod.outlook.com>
  0 siblings, 1 reply; 23+ messages in thread
From: Logan Gunthorpe @ 2020-10-06 23:59 UTC (permalink / raw)
  To: Chaitanya Kulkarni, linux-kernel, linux-nvme, linux-block, Omar Sandoval
  Cc: Sagi Grimberg, Stephen Bates



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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH blktests v2 05/11] nvme: Add common helpers for passthru tests
  2020-09-30 18:54 ` [PATCH blktests v2 05/11] nvme: Add common helpers for passthru tests Logan Gunthorpe
@ 2020-10-07  0:02   ` Chaitanya Kulkarni
  2020-10-07  0:13     ` Logan Gunthorpe
  0 siblings, 1 reply; 23+ messages in thread
From: Chaitanya Kulkarni @ 2020-10-07  0:02 UTC (permalink / raw)
  To: Logan Gunthorpe, linux-kernel, linux-nvme, linux-block, Omar Sandoval
  Cc: Sagi Grimberg, Stephen Bates

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'



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH blktests v2 04/11] nvme: Search for specific subsysnqn in _find_nvme_loop_dev
  2020-10-06 23:55   ` Chaitanya Kulkarni
@ 2020-10-07  0:10     ` Logan Gunthorpe
       [not found]       ` <BYAPR04MB49650C6419A84705D04FFE63860A0@BYAPR04MB4965.namprd04.prod.outlook.com>
  0 siblings, 1 reply; 23+ messages in thread
From: Logan Gunthorpe @ 2020-10-07  0:10 UTC (permalink / raw)
  To: Chaitanya Kulkarni, linux-kernel, linux-nvme, linux-block, Omar Sandoval
  Cc: Sagi Grimberg, Stephen Bates



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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH blktests v2 05/11] nvme: Add common helpers for passthru tests
  2020-10-07  0:02   ` Chaitanya Kulkarni
@ 2020-10-07  0:13     ` Logan Gunthorpe
  0 siblings, 0 replies; 23+ messages in thread
From: Logan Gunthorpe @ 2020-10-07  0:13 UTC (permalink / raw)
  To: Chaitanya Kulkarni, linux-kernel, linux-nvme, linux-block, Omar Sandoval
  Cc: Sagi Grimberg, Stephen Bates



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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH blktests v2 02/11] common/xfs: Create common helper to check for XFS support
  2020-10-06 23:51     ` Logan Gunthorpe
@ 2020-10-07  0:58       ` Chaitanya Kulkarni
  0 siblings, 0 replies; 23+ messages in thread
From: Chaitanya Kulkarni @ 2020-10-07  0:58 UTC (permalink / raw)
  To: Logan Gunthorpe, linux-kernel, linux-nvme, linux-block, Omar Sandoval
  Cc: Sagi Grimberg, Stephen Bates

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.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH blktests v2 03/11] common/xfs: Create common helper to verify block device with xfs
       [not found]       ` <BYAPR04MB49652338A4FE3805F9394A88860A0@BYAPR04MB4965.namprd04.prod.outlook.com>
@ 2020-10-07 15:53         ` Logan Gunthorpe
  0 siblings, 0 replies; 23+ messages in thread
From: Logan Gunthorpe @ 2020-10-07 15:53 UTC (permalink / raw)
  To: Chaitanya Kulkarni, linux-kernel, linux-nvme, linux-block, Omar Sandoval
  Cc: Sagi Grimberg, Stephen Bates



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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH blktests v2 04/11] nvme: Search for specific subsysnqn in _find_nvme_loop_dev
       [not found]       ` <BYAPR04MB49650C6419A84705D04FFE63860A0@BYAPR04MB4965.namprd04.prod.outlook.com>
@ 2020-10-07 15:55         ` Logan Gunthorpe
  0 siblings, 0 replies; 23+ messages in thread
From: Logan Gunthorpe @ 2020-10-07 15:55 UTC (permalink / raw)
  To: Chaitanya Kulkarni, linux-kernel, linux-nvme, linux-block, Omar Sandoval
  Cc: Sagi Grimberg, Stephen Bates



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


^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2020-10-07 15:55 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-30 18:54 [PATCH blktests v2 00/11] NVMe Target Passthru Block Tests Logan Gunthorpe
2020-09-30 18:54 ` [PATCH blktests v2 01/11] common/fio: Remove state file in common helper Logan Gunthorpe
2020-09-30 18:54 ` [PATCH blktests v2 02/11] common/xfs: Create common helper to check for XFS support Logan Gunthorpe
2020-10-06 23:44   ` Chaitanya Kulkarni
2020-10-06 23:51     ` Logan Gunthorpe
2020-10-07  0:58       ` Chaitanya Kulkarni
2020-09-30 18:54 ` [PATCH blktests v2 03/11] common/xfs: Create common helper to verify block device with xfs Logan Gunthorpe
2020-10-06 23:50   ` Chaitanya Kulkarni
2020-10-06 23:59     ` Logan Gunthorpe
     [not found]       ` <BYAPR04MB49652338A4FE3805F9394A88860A0@BYAPR04MB4965.namprd04.prod.outlook.com>
2020-10-07 15:53         ` Logan Gunthorpe
2020-09-30 18:54 ` [PATCH blktests v2 04/11] nvme: Search for specific subsysnqn in _find_nvme_loop_dev Logan Gunthorpe
2020-10-06 23:55   ` Chaitanya Kulkarni
2020-10-07  0:10     ` Logan Gunthorpe
     [not found]       ` <BYAPR04MB49650C6419A84705D04FFE63860A0@BYAPR04MB4965.namprd04.prod.outlook.com>
2020-10-07 15:55         ` Logan Gunthorpe
2020-09-30 18:54 ` [PATCH blktests v2 05/11] nvme: Add common helpers for passthru tests Logan Gunthorpe
2020-10-07  0:02   ` Chaitanya Kulkarni
2020-10-07  0:13     ` Logan Gunthorpe
2020-09-30 18:54 ` [PATCH blktests v2 06/11] nvme/033: Simple test to create and connect to a passthru target Logan Gunthorpe
2020-09-30 18:54 ` [PATCH blktests v2 07/11] nvme/034: Add test for passthru data verification Logan Gunthorpe
2020-09-30 18:54 ` [PATCH blktests v2 08/11] nvme/035: Add test to verify passthru controller with a filesystem Logan Gunthorpe
2020-09-30 18:54 ` [PATCH blktests v2 09/11] nvme/036: Add test for testing reset command on nvme-passthru Logan Gunthorpe
2020-09-30 18:54 ` [PATCH blktests v2 10/11] nvme/037: Add test which loops passthru connect and disconnect Logan Gunthorpe
2020-09-30 18:54 ` [PATCH blktests v2 11/11] nvme/038: Test removal of un-enabled subsystem and ports Logan Gunthorpe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).