[PATCH blktests v3 0/3] Introduce nvmet target setup/cleanup helpers

[PATCH blktests v3 0/3] Introduce nvmet target setup/cleanup helpers

[Daniel Wagner]

Addressed Sagi's feedback and dropped the explicit --blkdev=device argument from
the _nvmet_target_setup calls, as it is the default.



original cover letter:

Introduce helpers to setup nvmet targets. This is spin off from the refactoring
patches and the allowed_host patches [1].

Sagi suggested to record all resources allocated by nvmet_target_setup and then
later clean them up in nvmet_target_cleanup. I opted to figure out in
nvmet_target_cleanup what was allocated via the newly introdcuded _get_nvmet_ports
helper. The reason being, Hannes told me offline that he would like to add ANA
tests which will add some more ports to the subsystem. I hope with this
the code is more future proof.

BTW, while looking at this I saw that the passthru code is using the awkward
return value port when calling nvmet_passthru_target_setup. It seems some
more refactoring is in order...

[1] https://lore.kernel.org/linux-nvme/5h333eqhtw252sjw6axjewlb5bbb5ze7awekczxe3kie2lnhw6@manyer42khct/


changes

v3:
 - rebased/retested
 - use the default with _nvmet_target_setup 

v2:
 - drop local subsys variable in passthru tests
 - do not use port as handle in passthru tests
 - free port after unregistering from subsys
 - https://lore.kernel.org/linux-nvme/20230818141537.22332-1-dwagner@suse.de/

v1:
 - https://lore.kernel.org/linux-nvme/20230818095744.24619-1-dwagner@suse.de/

Daniel Wagner (3):
  nvme/{033,034,035,036}: use default subsysnqn variable directly
  nvme/{033,034,035,036,37}: drop port handle between passthru target
    setup and cleanup
  nvme: introduce nvmet_target_{setup/cleanup} common code

 tests/nvme/003 | 14 ++-----
 tests/nvme/004 | 21 ++---------
 tests/nvme/005 | 20 +---------
 tests/nvme/006 | 19 +---------
 tests/nvme/007 | 14 +------
 tests/nvme/008 | 21 +----------
 tests/nvme/009 | 16 +-------
 tests/nvme/010 | 21 +----------
 tests/nvme/011 | 16 +-------
 tests/nvme/012 | 21 +----------
 tests/nvme/013 | 16 +-------
 tests/nvme/014 | 21 +----------
 tests/nvme/015 | 16 +-------
 tests/nvme/018 | 16 +-------
 tests/nvme/019 | 21 +----------
 tests/nvme/020 | 16 +-------
 tests/nvme/021 | 16 +-------
 tests/nvme/022 | 16 +-------
 tests/nvme/023 | 21 +----------
 tests/nvme/024 | 16 +-------
 tests/nvme/025 | 16 +-------
 tests/nvme/026 | 16 +-------
 tests/nvme/027 | 17 ++-------
 tests/nvme/028 | 17 ++-------
 tests/nvme/029 | 21 +----------
 tests/nvme/033 | 10 ++---
 tests/nvme/034 | 10 ++---
 tests/nvme/035 | 10 ++---
 tests/nvme/036 | 12 +++---
 tests/nvme/037 |  5 +--
 tests/nvme/040 | 19 +---------
 tests/nvme/041 | 18 +--------
 tests/nvme/042 | 17 +--------
 tests/nvme/043 | 17 +--------
 tests/nvme/044 | 19 ++--------
 tests/nvme/045 | 18 ++-------
 tests/nvme/047 | 21 +----------
 tests/nvme/048 | 17 +--------
 tests/nvme/rc  | 99 +++++++++++++++++++++++++++++++++++++++++++++++---
 39 files changed, 184 insertions(+), 553 deletions(-)

-- 
2.41.0

[PATCH blktests v3 1/3] nvme/{033,034,035,036}: use default subsysnqn variable directly

[Daniel Wagner]

There is no need to introduce an extra local variable when it
uses the default subsysnqn anyway.

Signed-off-by: Daniel Wagner <dwagner@suse.de>
---
 tests/nvme/033 |  9 ++++-----
 tests/nvme/034 |  9 ++++-----
 tests/nvme/035 |  9 ++++-----
 tests/nvme/036 | 11 +++++------
 4 files changed, 17 insertions(+), 21 deletions(-)

diff --git a/tests/nvme/033 b/tests/nvme/033
index 46a520ae01fa..d924883460c2 100755
--- a/tests/nvme/033
+++ b/tests/nvme/033
@@ -49,18 +49,17 @@ test_device() {
 
 	_setup_nvmet
 
-	local subsys="${def_subsysnqn}"
 	local nsdev
 	local port
 
-	port=$(_nvmet_passthru_target_setup "${subsys}")
+	port=$(_nvmet_passthru_target_setup "${def_subsysnqn}")
 
-	nsdev=$(_nvmet_passthru_target_connect "${nvme_trtype}" "${subsys}")
+	nsdev=$(_nvmet_passthru_target_connect "${nvme_trtype}" "${def_subsysnqn}")
 
 	compare_dev_info "${nsdev}"
 
-	_nvme_disconnect_subsys "${subsys}"
-	_nvmet_passthru_target_cleanup "${port}" "${subsys}"
+	_nvme_disconnect_subsys "${def_subsysnqn}"
+	_nvmet_passthru_target_cleanup "${port}" "${def_subsysnqn}"
 
 	echo "Test complete"
 }
diff --git a/tests/nvme/034 b/tests/nvme/034
index 3c65d92cbaea..e79eef5e756d 100755
--- a/tests/nvme/034
+++ b/tests/nvme/034
@@ -19,18 +19,17 @@ test_device() {
 
 	_setup_nvmet
 
-	local subsys="${def_subsysnqn}"
 	local ctrldev
 	local nsdev
 	local port
 
-	port=$(_nvmet_passthru_target_setup "${subsys}")
-	nsdev=$(_nvmet_passthru_target_connect "${nvme_trtype}" "${subsys}")
+	port=$(_nvmet_passthru_target_setup "${def_subsysnqn}")
+	nsdev=$(_nvmet_passthru_target_connect "${nvme_trtype}" "${def_subsysnqn}")
 
 	_run_fio_verify_io --size="${nvme_img_size}" --filename="${nsdev}"
 
-	_nvme_disconnect_subsys "${subsys}"
-	_nvmet_passthru_target_cleanup "${port}" "${subsys}"
+	_nvme_disconnect_subsys "${def_subsysnqn}"
+	_nvmet_passthru_target_cleanup "${port}" "${def_subsysnqn}"
 
 	echo "Test complete"
 }
diff --git a/tests/nvme/035 b/tests/nvme/035
index c705d9cf25a4..f0dfc92ceeea 100755
--- a/tests/nvme/035
+++ b/tests/nvme/035
@@ -25,18 +25,17 @@ test_device() {
 
 	_setup_nvmet
 
-	local subsys="${def_subsysnqn}"
 	local ctrldev
 	local nsdev
 	local port
 
-	port=$(_nvmet_passthru_target_setup "${subsys}")
-	nsdev=$(_nvmet_passthru_target_connect "${nvme_trtype}" "${subsys}")
+	port=$(_nvmet_passthru_target_setup "${def_subsysnqn}")
+	nsdev=$(_nvmet_passthru_target_connect "${nvme_trtype}" "${def_subsysnqn}")
 
 	_xfs_run_fio_verify_io "${nsdev}" "${nvme_img_size}"
 
-	_nvme_disconnect_subsys "${subsys}"
-	_nvmet_passthru_target_cleanup "${port}" "${subsys}"
+	_nvme_disconnect_subsys "${def_subsysnqn}"
+	_nvmet_passthru_target_cleanup "${port}" "${def_subsysnqn}"
 
 	echo "Test complete"
 }
diff --git a/tests/nvme/036 b/tests/nvme/036
index 2e933cc41928..61b9e2309da7 100755
--- a/tests/nvme/036
+++ b/tests/nvme/036
@@ -18,21 +18,20 @@ test_device() {
 
 	_setup_nvmet
 
-	local subsys="${def_subsysnqn}"
 	local ctrldev
 	local port
 
-	port=$(_nvmet_passthru_target_setup "${subsys}")
-	nsdev=$(_nvmet_passthru_target_connect "${nvme_trtype}" "${subsys}")
+	port=$(_nvmet_passthru_target_setup "${def_subsysnqn}")
+	nsdev=$(_nvmet_passthru_target_connect "${nvme_trtype}" "${def_subsysnqn}")
 
-	ctrldev=$(_find_nvme_dev "${subsys}")
+	ctrldev=$(_find_nvme_dev "${def_subsysnqn}")
 
 	if ! nvme reset "/dev/${ctrldev}" >> "$FULL" 2>&1; then
 		echo "ERROR: reset failed"
 	fi
 
-	_nvme_disconnect_subsys "${subsys}"
-	_nvmet_passthru_target_cleanup "${port}" "${subsys}"
+	_nvme_disconnect_subsys "${def_subsysnqn}"
+	_nvmet_passthru_target_cleanup "${port}" "${def_subsysnqn}"
 
 	echo "Test complete"
 }
-- 
2.41.0

[PATCH blktests v3 2/3] nvme/{033,034,035,036,37}: drop port handle between passthru target setup and cleanup

[Daniel Wagner]

The passthru nvmet setup and cleanup helpers are using the port as
handle to track resources.

Instead returning the port from the setup call, we figure out in the
cleanup code which resources have been allocated. This avoids passing
around awkward handles.

Signed-off-by: Daniel Wagner <dwagner@suse.de>
---
 tests/nvme/033 |  5 ++---
 tests/nvme/034 |  5 ++---
 tests/nvme/035 |  5 ++---
 tests/nvme/036 |  5 ++---
 tests/nvme/037 |  5 ++---
 tests/nvme/rc  | 30 ++++++++++++++++++++++++------
 6 files changed, 34 insertions(+), 21 deletions(-)

diff --git a/tests/nvme/033 b/tests/nvme/033
index d924883460c2..6cc4f57e6d60 100755
--- a/tests/nvme/033
+++ b/tests/nvme/033
@@ -50,16 +50,15 @@ test_device() {
 	_setup_nvmet
 
 	local nsdev
-	local port
 
-	port=$(_nvmet_passthru_target_setup "${def_subsysnqn}")
+	_nvmet_passthru_target_setup "${def_subsysnqn}"
 
 	nsdev=$(_nvmet_passthru_target_connect "${nvme_trtype}" "${def_subsysnqn}")
 
 	compare_dev_info "${nsdev}"
 
 	_nvme_disconnect_subsys "${def_subsysnqn}"
-	_nvmet_passthru_target_cleanup "${port}" "${def_subsysnqn}"
+	_nvmet_passthru_target_cleanup "${def_subsysnqn}"
 
 	echo "Test complete"
 }
diff --git a/tests/nvme/034 b/tests/nvme/034
index e79eef5e756d..3bd1c3ad2f61 100755
--- a/tests/nvme/034
+++ b/tests/nvme/034
@@ -21,15 +21,14 @@ test_device() {
 
 	local ctrldev
 	local nsdev
-	local port
 
-	port=$(_nvmet_passthru_target_setup "${def_subsysnqn}")
+	_nvmet_passthru_target_setup "${def_subsysnqn}"
 	nsdev=$(_nvmet_passthru_target_connect "${nvme_trtype}" "${def_subsysnqn}")
 
 	_run_fio_verify_io --size="${nvme_img_size}" --filename="${nsdev}"
 
 	_nvme_disconnect_subsys "${def_subsysnqn}"
-	_nvmet_passthru_target_cleanup "${port}" "${def_subsysnqn}"
+	_nvmet_passthru_target_cleanup "${def_subsysnqn}"
 
 	echo "Test complete"
 }
diff --git a/tests/nvme/035 b/tests/nvme/035
index f0dfc92ceeea..712fe1dbcfb8 100755
--- a/tests/nvme/035
+++ b/tests/nvme/035
@@ -27,15 +27,14 @@ test_device() {
 
 	local ctrldev
 	local nsdev
-	local port
 
-	port=$(_nvmet_passthru_target_setup "${def_subsysnqn}")
+	_nvmet_passthru_target_setup "${def_subsysnqn}"
 	nsdev=$(_nvmet_passthru_target_connect "${nvme_trtype}" "${def_subsysnqn}")
 
 	_xfs_run_fio_verify_io "${nsdev}" "${nvme_img_size}"
 
 	_nvme_disconnect_subsys "${def_subsysnqn}"
-	_nvmet_passthru_target_cleanup "${port}" "${def_subsysnqn}"
+	_nvmet_passthru_target_cleanup "${def_subsysnqn}"
 
 	echo "Test complete"
 }
diff --git a/tests/nvme/036 b/tests/nvme/036
index 61b9e2309da7..89ccd256a67c 100755
--- a/tests/nvme/036
+++ b/tests/nvme/036
@@ -19,9 +19,8 @@ test_device() {
 	_setup_nvmet
 
 	local ctrldev
-	local port
 
-	port=$(_nvmet_passthru_target_setup "${def_subsysnqn}")
+	_nvmet_passthru_target_setup "${def_subsysnqn}"
 	nsdev=$(_nvmet_passthru_target_connect "${nvme_trtype}" "${def_subsysnqn}")
 
 	ctrldev=$(_find_nvme_dev "${def_subsysnqn}")
@@ -31,7 +30,7 @@ test_device() {
 	fi
 
 	_nvme_disconnect_subsys "${def_subsysnqn}"
-	_nvmet_passthru_target_cleanup "${port}" "${def_subsysnqn}"
+	_nvmet_passthru_target_cleanup "${def_subsysnqn}"
 
 	echo "Test complete"
 }
diff --git a/tests/nvme/037 b/tests/nvme/037
index 5a78444b7e78..a2815b3ff2d7 100755
--- a/tests/nvme/037
+++ b/tests/nvme/037
@@ -20,15 +20,14 @@ test_device() {
 	local subsys="blktests-subsystem-"
 	local iterations=10
 	local ctrldev
-	local port
 
 	for ((i = 0; i < iterations; i++)); do
-		port=$(_nvmet_passthru_target_setup "${subsys}${i}")
+		_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}"
+		_nvmet_passthru_target_cleanup "${subsys}${i}"
 	done
 
 	echo "Test complete"
diff --git a/tests/nvme/rc b/tests/nvme/rc
index 10a0d6d27f02..fdffc07da34a 100644
--- a/tests/nvme/rc
+++ b/tests/nvme/rc
@@ -745,6 +745,19 @@ _remove_nvmet_subsystem_from_port() {
 	rm "${NVMET_CFS}/ports/${port}/subsystems/${nvmet_subsystem}"
 }
 
+_get_nvmet_ports() {
+	local nvmet_subsystem="$1"
+	local -n nvmet_ports="$2"
+	local cfs_path="${NVMET_CFS}/ports"
+	local sarg
+
+	sarg="s;^${cfs_path}/\([0-9]\+\)/subsystems/${nvmet_subsystem}$;\1;p"
+
+	for path in "${cfs_path}/"*"/subsystems/${nvmet_subsystem}"; do
+		nvmet_ports+=("$(echo "${path}" | sed -n -s "${sarg}")")
+	done
+}
+
 _set_nvmet_hostkey() {
 	local nvmet_hostnqn="$1"
 	local nvmet_hostkey="$2"
@@ -807,13 +820,12 @@ _find_nvme_passthru_loop_dev() {
 
 _nvmet_passthru_target_setup() {
 	local subsys_name=$1
+	local port
 
 	_create_nvmet_passthru "${subsys_name}"
 	port="$(_create_nvmet_port "${nvme_trtype}")"
 	_add_nvmet_subsys_to_port "${port}" "${subsys_name}"
 	_create_nvmet_host "${subsys_name}" "${def_hostnqn}"
-
-	echo "$port"
 }
 
 _nvmet_passthru_target_connect() {
@@ -832,11 +844,17 @@ _nvmet_passthru_target_connect() {
 }
 
 _nvmet_passthru_target_cleanup() {
-	local port=$1
-	local subsys_name=$2
+	local subsys_name=$1
+	local ports
+	local port
+
+	_get_nvmet_ports "${subsys_name}" ports
+
+	for port in "${ports[@]}"; do
+		_remove_nvmet_subsystem_from_port "${port}" "${subsys_name}"
+		_remove_nvmet_port "${port}"
+	done
 
-	_remove_nvmet_subsystem_from_port "${port}" "${subsys_name}"
-	_remove_nvmet_port "${port}"
 	_remove_nvmet_passhtru "${subsys_name}"
 	_remove_nvmet_host "${def_hostnqn}"
 }
-- 
2.41.0

[PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code

[Daniel Wagner]

Almost all fabric tests have the identically code for
setting up and cleaning up the target side. Introduce
two new helpers.

Signed-off-by: Daniel Wagner <dwagner@suse.de>
---
 tests/nvme/003 | 14 +++-------
 tests/nvme/004 | 21 +++------------
 tests/nvme/005 | 20 ++-------------
 tests/nvme/006 | 19 ++------------
 tests/nvme/007 | 14 ++--------
 tests/nvme/008 | 21 ++-------------
 tests/nvme/009 | 16 ++----------
 tests/nvme/010 | 21 ++-------------
 tests/nvme/011 | 16 ++----------
 tests/nvme/012 | 21 ++-------------
 tests/nvme/013 | 16 ++----------
 tests/nvme/014 | 21 ++-------------
 tests/nvme/015 | 16 ++----------
 tests/nvme/018 | 16 ++----------
 tests/nvme/019 | 21 ++-------------
 tests/nvme/020 | 16 ++----------
 tests/nvme/021 | 16 ++----------
 tests/nvme/022 | 16 ++----------
 tests/nvme/023 | 21 ++-------------
 tests/nvme/024 | 16 ++----------
 tests/nvme/025 | 16 ++----------
 tests/nvme/026 | 16 ++----------
 tests/nvme/027 | 17 +++----------
 tests/nvme/028 | 17 +++----------
 tests/nvme/029 | 21 ++-------------
 tests/nvme/040 | 19 ++------------
 tests/nvme/041 | 18 ++-----------
 tests/nvme/042 | 17 ++-----------
 tests/nvme/043 | 17 ++-----------
 tests/nvme/044 | 19 +++-----------
 tests/nvme/045 | 18 +++----------
 tests/nvme/047 | 21 ++-------------
 tests/nvme/048 | 17 ++-----------
 tests/nvme/rc  | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++
 34 files changed, 141 insertions(+), 519 deletions(-)

diff --git a/tests/nvme/003 b/tests/nvme/003
index 71b82ce758a3..b5ea2720100e 100755
--- a/tests/nvme/003
+++ b/tests/nvme/003
@@ -22,15 +22,8 @@ test() {
 
 	_setup_nvmet
 
-	local loop_dev
-	local port
 
-	port="$(_create_nvmet_port "${nvme_trtype}")"
-
-	loop_dev="$(losetup -f)"
-
-	_create_nvmet_subsystem "${def_subsysnqn}" "${loop_dev}"
-	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
+	_nvmet_target_setup
 
 	_nvme_connect_subsys "${nvme_trtype}" nqn.2014-08.org.nvmexpress.discovery
 
@@ -46,9 +39,8 @@ test() {
 	fi
 
 	_nvme_disconnect_subsys nqn.2014-08.org.nvmexpress.discovery
-	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
-	_remove_nvmet_subsystem "${def_subsysnqn}"
-	_remove_nvmet_port "${port}"
+
+	_nvmet_target_cleanup
 
 	echo "Test complete"
 }
diff --git a/tests/nvme/004 b/tests/nvme/004
index 697c758d3059..31af8737857b 100755
--- a/tests/nvme/004
+++ b/tests/nvme/004
@@ -22,19 +22,8 @@ test() {
 
 	_setup_nvmet
 
-	local port
-	local loop_dev
 
-	port="$(_create_nvmet_port "${nvme_trtype}")"
-
-	truncate -s "${nvme_img_size}" "${def_file_path}"
-
-	loop_dev="$(losetup -f --show "${def_file_path}")"
-
-	_create_nvmet_subsystem "${def_subsysnqn}" "${loop_dev}" \
-		"${def_subsys_uuid}"
-	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
-	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
+	_nvmet_target_setup
 
 	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
 
@@ -44,12 +33,8 @@ test() {
 	cat "/sys/block/${nvmedev}n1/wwid"
 
 	_nvme_disconnect_subsys ${def_subsysnqn}
-	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
-	_remove_nvmet_subsystem "${def_subsysnqn}"
-	_remove_nvmet_port "${port}"
-	_remove_nvmet_host "${def_hostnqn}"
-	losetup -d "$loop_dev"
-	rm "${def_file_path}"
+
+	_nvmet_target_cleanup
 
 	echo "Test complete"
 }
diff --git a/tests/nvme/005 b/tests/nvme/005
index 4ca87ff48016..f9956e960a56 100755
--- a/tests/nvme/005
+++ b/tests/nvme/005
@@ -21,20 +21,9 @@ test() {
 
 	_setup_nvmet
 
-	local port
-	local loop_dev
 	local nvmedev
 
-	port="$(_create_nvmet_port "${nvme_trtype}")"
-
-	truncate -s "${nvme_img_size}" "${def_file_path}"
-
-	loop_dev="$(losetup -f --show "${def_file_path}")"
-
-	_create_nvmet_subsystem "${def_subsysnqn}" "${loop_dev}" \
-		"${def_subsys_uuid}"
-	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
-	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
+	_nvmet_target_setup
 
 	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
 
@@ -43,13 +32,8 @@ test() {
 	echo 1 > "/sys/class/nvme/${nvmedev}/reset_controller"
 
 	_nvme_disconnect_ctrl "${nvmedev}"
-	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
-	_remove_nvmet_port "${port}"
-	_remove_nvmet_subsystem "${def_subsysnqn}"
-	_remove_nvmet_host "${def_hostnqn}"
 
-	losetup -d "$loop_dev"
-	rm "${def_file_path}"
+	_nvmet_target_cleanup
 
 	echo "Test complete"
 }
diff --git a/tests/nvme/006 b/tests/nvme/006
index 910204aaeb90..d85f64b702eb 100755
--- a/tests/nvme/006
+++ b/tests/nvme/006
@@ -20,25 +20,10 @@ test() {
 
 	_setup_nvmet
 
-	local port
-	local loop_dev
 
-	truncate -s "${nvme_img_size}" "${def_file_path}"
+	_nvmet_target_setup
 
-	loop_dev="$(losetup -f --show "${def_file_path}")"
-
-	_create_nvmet_subsystem "${def_subsysnqn}" "${loop_dev}" \
-		"${def_subsys_uuid}"
-	port="$(_create_nvmet_port "${nvme_trtype}")"
-	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
-
-	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
-	_remove_nvmet_subsystem "${def_subsysnqn}"
-	_remove_nvmet_port "${port}"
-
-	losetup -d "$loop_dev"
-
-	rm "${def_file_path}"
+	_nvmet_target_cleanup
 
 	echo "Test complete"
 }
diff --git a/tests/nvme/007 b/tests/nvme/007
index db00bdcc2d08..feac5060a950 100755
--- a/tests/nvme/007
+++ b/tests/nvme/007
@@ -19,20 +19,10 @@ test() {
 
 	_setup_nvmet
 
-	local port
 
-	truncate -s "${nvme_img_size}" "${def_file_path}"
+	_nvmet_target_setup --blkdev=file
 
-	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}" \
-		"${def_subsys_uuid}"
-	port="$(_create_nvmet_port "${nvme_trtype}")"
-	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
-
-	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
-	_remove_nvmet_subsystem "${def_subsysnqn}"
-	_remove_nvmet_port "${port}"
-
-	rm "${def_file_path}"
+	_nvmet_target_cleanup
 
 	echo "Test complete"
 }
diff --git a/tests/nvme/008 b/tests/nvme/008
index bd5e10fbfb99..f4b45b2f1c11 100755
--- a/tests/nvme/008
+++ b/tests/nvme/008
@@ -20,19 +20,9 @@ test() {
 
 	_setup_nvmet
 
-	local port
 	local nvmedev
-	local loop_dev
 
-	truncate -s "${nvme_img_size}" "${def_file_path}"
-
-	loop_dev="$(losetup -f --show "${def_file_path}")"
-
-	_create_nvmet_subsystem "${def_subsysnqn}" "${loop_dev}" \
-		"${def_subsys_uuid}"
-	port="$(_create_nvmet_port "${nvme_trtype}")"
-	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
-	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
+	_nvmet_target_setup
 
 	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
 
@@ -42,14 +32,7 @@ test() {
 
 	_nvme_disconnect_subsys "${def_subsysnqn}"
 
-	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
-	_remove_nvmet_subsystem "${def_subsysnqn}"
-	_remove_nvmet_port "${port}"
-	_remove_nvmet_host "${def_hostnqn}"
-
-	losetup -d "${loop_dev}"
-
-	rm "${def_file_path}"
+	_nvmet_target_cleanup
 
 	echo "Test complete"
 }
diff --git a/tests/nvme/009 b/tests/nvme/009
index c9a4b57ac288..63614c91a96d 100755
--- a/tests/nvme/009
+++ b/tests/nvme/009
@@ -19,16 +19,9 @@ test() {
 
 	_setup_nvmet
 
-	local port
 	local nvmedev
 
-	truncate -s "${nvme_img_size}" "${def_file_path}"
-
-	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}" \
-		"${def_subsys_uuid}"
-	port="$(_create_nvmet_port "${nvme_trtype}")"
-	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
-	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
+	_nvmet_target_setup --blkdev=file
 
 	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
 
@@ -38,12 +31,7 @@ test() {
 
 	_nvme_disconnect_subsys "${def_subsysnqn}"
 
-	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
-	_remove_nvmet_subsystem "${def_subsysnqn}"
-	_remove_nvmet_port "${port}"
-	_remove_nvmet_host "${def_hostnqn}"
-
-	rm "${def_file_path}"
+	_nvmet_target_cleanup
 
 	echo "Test complete"
 }
diff --git a/tests/nvme/010 b/tests/nvme/010
index 19bb7f3fc7a7..e782a9bb06f3 100755
--- a/tests/nvme/010
+++ b/tests/nvme/010
@@ -20,19 +20,9 @@ test() {
 
 	_setup_nvmet
 
-	local port
 	local nvmedev
-	local loop_dev
 
-	truncate -s "${nvme_img_size}" "${def_file_path}"
-
-	loop_dev="$(losetup -f --show "${def_file_path}")"
-
-	_create_nvmet_subsystem "${def_subsysnqn}" "${loop_dev}" \
-		"${def_subsys_uuid}"
-	port="$(_create_nvmet_port "${nvme_trtype}")"
-	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
-	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
+	_nvmet_target_setup
 
 	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
 
@@ -45,14 +35,7 @@ test() {
 
 	_nvme_disconnect_subsys "${def_subsysnqn}"
 
-	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
-	_remove_nvmet_subsystem "${def_subsysnqn}"
-	_remove_nvmet_port "${port}"
-	_remove_nvmet_host "${def_hostnqn}"
-
-	losetup -d "${loop_dev}"
-
-	rm "${def_file_path}"
+	_nvmet_target_cleanup
 
 	echo "Test complete"
 }
diff --git a/tests/nvme/011 b/tests/nvme/011
index 0e54c2588bc8..7329e0505f59 100755
--- a/tests/nvme/011
+++ b/tests/nvme/011
@@ -20,16 +20,9 @@ test() {
 
 	_setup_nvmet
 
-	local port
 	local nvmedev
 
-	truncate -s "${nvme_img_size}" "${def_file_path}"
-
-	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}" \
-		"${def_subsys_uuid}"
-	port="$(_create_nvmet_port "${nvme_trtype}")"
-	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
-	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
+	_nvmet_target_setup --blkdev=file
 
 	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
 
@@ -42,12 +35,7 @@ test() {
 
 	_nvme_disconnect_subsys "${def_subsysnqn}"
 
-	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
-	_remove_nvmet_subsystem "${def_subsysnqn}"
-	_remove_nvmet_port "${port}"
-	_remove_nvmet_host "${def_hostnqn}"
-
-	rm "${def_file_path}"
+	_nvmet_target_cleanup
 
 	echo "Test complete"
 }
diff --git a/tests/nvme/012 b/tests/nvme/012
index c6b82c821bf2..6072eed3532a 100755
--- a/tests/nvme/012
+++ b/tests/nvme/012
@@ -24,19 +24,9 @@ test() {
 
 	_setup_nvmet
 
-	local port
 	local nvmedev
-	local loop_dev
 
-	truncate -s "${nvme_img_size}" "${def_file_path}"
-
-	loop_dev="$(losetup -f --show "${def_file_path}")"
-
-	_create_nvmet_subsystem "${def_subsysnqn}" "${loop_dev}" \
-		"${def_subsys_uuid}"
-	port="$(_create_nvmet_port "${nvme_trtype}")"
-	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
-	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
+	_nvmet_target_setup
 
 	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
 
@@ -48,14 +38,7 @@ test() {
 
 	_nvme_disconnect_subsys "${def_subsysnqn}"
 
-	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
-	_remove_nvmet_subsystem "${def_subsysnqn}"
-	_remove_nvmet_port "${port}"
-	_remove_nvmet_host "${def_hostnqn}"
-
-	losetup -d "${loop_dev}"
-
-	rm "${def_file_path}"
+	_nvmet_target_cleanup
 
 	echo "Test complete"
 }
diff --git a/tests/nvme/013 b/tests/nvme/013
index 441db7477d75..c9be60675cc4 100755
--- a/tests/nvme/013
+++ b/tests/nvme/013
@@ -23,16 +23,9 @@ test() {
 
 	_setup_nvmet
 
-	local port
 	local nvmedev
 
-	truncate -s "${nvme_img_size}" "${def_file_path}"
-
-	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}" \
-		"${def_subsys_uuid}"
-	port="$(_create_nvmet_port "${nvme_trtype}")"
-	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
-	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
+	_nvmet_target_setup --blkdev=file
 
 	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
 
@@ -44,12 +37,7 @@ test() {
 
 	_nvme_disconnect_subsys "${def_subsysnqn}"
 
-	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
-	_remove_nvmet_subsystem "${def_subsysnqn}"
-	_remove_nvmet_port "${port}"
-	_remove_nvmet_host "${def_hostnqn}"
-
-	rm "${def_file_path}"
+	_nvmet_target_cleanup
 
 	echo "Test complete"
 }
diff --git a/tests/nvme/014 b/tests/nvme/014
index 3656f9399687..d49e8f3cce4d 100755
--- a/tests/nvme/014
+++ b/tests/nvme/014
@@ -20,22 +20,12 @@ test() {
 
 	_setup_nvmet
 
-	local port
 	local nvmedev
-	local loop_dev
 	local size
 	local bs
 	local count
 
-	truncate -s "${nvme_img_size}" "${def_file_path}"
-
-	loop_dev="$(losetup -f --show "${def_file_path}")"
-
-	_create_nvmet_subsystem "${def_subsysnqn}" "${loop_dev}" \
-		 "${def_subsys_uuid}"
-	port="$(_create_nvmet_port "${nvme_trtype}")"
-	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
-	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
+	_nvmet_target_setup
 
 	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
 
@@ -54,14 +44,7 @@ test() {
 
 	_nvme_disconnect_subsys "${def_subsysnqn}"
 
-	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
-	_remove_nvmet_subsystem "${def_subsysnqn}"
-	_remove_nvmet_port "${port}"
-	_remove_nvmet_host "${def_hostnqn}"
-
-	losetup -d "${loop_dev}"
-
-	rm "${def_file_path}"
+	_nvmet_target_cleanup
 
 	echo "Test complete"
 }
diff --git a/tests/nvme/015 b/tests/nvme/015
index bc04e39c628c..b418d785ab27 100755
--- a/tests/nvme/015
+++ b/tests/nvme/015
@@ -20,19 +20,12 @@ test() {
 
 	_setup_nvmet
 
-	local port
 	local nvmedev
 	local size
 	local bs
 	local count
 
-	truncate -s "${nvme_img_size}" "${def_file_path}"
-
-	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}" \
-		 "${def_subsys_uuid}"
-	port="$(_create_nvmet_port "${nvme_trtype}")"
-	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
-	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
+	_nvmet_target_setup --blkdev=file
 
 	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
 
@@ -51,12 +44,7 @@ test() {
 
 	_nvme_disconnect_subsys "${def_subsysnqn}"
 
-	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
-	_remove_nvmet_subsystem "${def_subsysnqn}"
-	_remove_nvmet_port "${port}"
-	_remove_nvmet_host "${def_hostnqn}"
-
-	rm "${def_file_path}"
+	_nvmet_target_cleanup
 
 	echo "Test complete"
 }
diff --git a/tests/nvme/018 b/tests/nvme/018
index 68729c3cb070..19e439f3f3e0 100755
--- a/tests/nvme/018
+++ b/tests/nvme/018
@@ -21,16 +21,9 @@ test() {
 
 	_setup_nvmet
 
-	local port
 	local nvmedev
 
-	truncate -s "${nvme_img_size}" "${def_file_path}"
-
-	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}" \
-		 "${def_subsys_uuid}"
-	port="$(_create_nvmet_port "${nvme_trtype}")"
-	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
-	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
+	_nvmet_target_setup
 
 	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
 
@@ -48,12 +41,7 @@ test() {
 
 	_nvme_disconnect_subsys "${def_subsysnqn}"
 
-	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
-	_remove_nvmet_subsystem "${def_subsysnqn}"
-	_remove_nvmet_port "${port}"
-	_remove_nvmet_host "${def_hostnqn}"
-
-	rm "${def_file_path}"
+	_nvmet_target_cleanup
 
 	echo "Test complete"
 }
diff --git a/tests/nvme/019 b/tests/nvme/019
index 33a25d52e9fd..15e98c40134f 100755
--- a/tests/nvme/019
+++ b/tests/nvme/019
@@ -20,21 +20,11 @@ test() {
 
 	_setup_nvmet
 
-	local port
 	local nvmedev
-	local loop_dev
 	local nblk_range="10,10,10,10,10,10,10,10,10,10"
 	local sblk_range="100,200,300,400,500,600,700,800,900,1000"
 
-	truncate -s "${nvme_img_size}" "${def_file_path}"
-
-	loop_dev="$(losetup -f --show "${def_file_path}")"
-
-	_create_nvmet_subsystem "${def_subsysnqn}" "${loop_dev}" \
-		"${def_subsys_uuid}"
-	port="$(_create_nvmet_port "${nvme_trtype}")"
-	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
-	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
+	_nvmet_target_setup
 
 	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
 
@@ -46,14 +36,7 @@ test() {
 
 	_nvme_disconnect_subsys "${def_subsysnqn}"
 
-	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
-	_remove_nvmet_subsystem "${def_subsysnqn}"
-	_remove_nvmet_port "${port}"
-	_remove_nvmet_host "${def_hostnqn}"
-
-	losetup -d "${loop_dev}"
-
-	rm -f "${def_file_path}"
+	_nvmet_target_cleanup
 
 	echo "Test complete"
 }
diff --git a/tests/nvme/020 b/tests/nvme/020
index f436cdc8b262..aae40e7131e0 100755
--- a/tests/nvme/020
+++ b/tests/nvme/020
@@ -19,18 +19,11 @@ test() {
 
 	_setup_nvmet
 
-	local port
 	local nvmedev
 	local nblk_range="10,10,10,10,10,10,10,10,10,10"
 	local sblk_range="100,200,300,400,500,600,700,800,900,1000"
 
-	truncate -s "${nvme_img_size}" "${def_file_path}"
-
-	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}" \
-		"${def_subsys_uuid}"
-	port="$(_create_nvmet_port "${nvme_trtype}")"
-	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
-	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
+	_nvmet_target_setup --blkdev=file
 
 	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
 
@@ -42,12 +35,7 @@ test() {
 
 	_nvme_disconnect_subsys "${def_subsysnqn}"
 
-	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
-	_remove_nvmet_subsystem "${def_subsysnqn}"
-	_remove_nvmet_port "${port}"
-	_remove_nvmet_host "${def_hostnqn}"
-
-	rm -f "${def_file_path}"
+	_nvmet_target_cleanup
 
 	echo "Test complete"
 }
diff --git a/tests/nvme/021 b/tests/nvme/021
index 5043fe4916be..f9bed1546307 100755
--- a/tests/nvme/021
+++ b/tests/nvme/021
@@ -20,16 +20,9 @@ test() {
 
 	_setup_nvmet
 
-	local port
 	local nvmedev
 
-	truncate -s "${nvme_img_size}" "${def_file_path}"
-
-	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}" \
-		"${def_subsys_uuid}"
-	port="$(_create_nvmet_port "${nvme_trtype}")"
-	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
-	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
+	_nvmet_target_setup --blkdev=file
 
 	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
 
@@ -43,12 +36,7 @@ test() {
 
 	_nvme_disconnect_subsys "${def_subsysnqn}" >> "$FULL" 2>&1
 
-	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
-	_remove_nvmet_subsystem "${def_subsysnqn}"
-	_remove_nvmet_port "${port}"
-	_remove_nvmet_host "${def_hostnqn}"
-
-	rm -f "${def_file_path}"
+	_nvmet_target_cleanup
 
 	echo "Test complete"
 }
diff --git a/tests/nvme/022 b/tests/nvme/022
index 8b6f610c4894..e3e67b0996df 100755
--- a/tests/nvme/022
+++ b/tests/nvme/022
@@ -20,16 +20,9 @@ test() {
 
 	_setup_nvmet
 
-	local port
 	local nvmedev
 
-	truncate -s "${nvme_img_size}" "${def_file_path}"
-
-	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}" \
-		"${def_subsys_uuid}"
-	port="$(_create_nvmet_port "${nvme_trtype}")"
-	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
-	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
+	_nvmet_target_setup --blkdev=file
 
 	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
 
@@ -43,12 +36,7 @@ test() {
 
 	_nvme_disconnect_subsys "${def_subsysnqn}" >> "$FULL" 2>&1
 
-	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
-	_remove_nvmet_subsystem "${def_subsysnqn}"
-	_remove_nvmet_port "${port}"
-	_remove_nvmet_host "${def_hostnqn}"
-
-	rm -f "${def_file_path}"
+	_nvmet_target_cleanup
 
 	echo "Test complete"
 }
diff --git a/tests/nvme/023 b/tests/nvme/023
index 90af0338e81f..c8d1e4619822 100755
--- a/tests/nvme/023
+++ b/tests/nvme/023
@@ -20,19 +20,9 @@ test() {
 
 	_setup_nvmet
 
-	local port
 	local nvmedev
-	local loop_dev
 
-	truncate -s "${nvme_img_size}" "${def_file_path}"
-
-	loop_dev="$(losetup -f --show "${def_file_path}")"
-
-	_create_nvmet_subsystem "${def_subsysnqn}" "${loop_dev}" \
-		"${def_subsys_uuid}"
-	port="$(_create_nvmet_port "${nvme_trtype}")"
-	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
-	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
+	_nvmet_target_setup
 
 	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
 
@@ -46,14 +36,7 @@ test() {
 
 	_nvme_disconnect_subsys "${def_subsysnqn}" >> "$FULL" 2>&1
 
-	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
-	_remove_nvmet_subsystem "${def_subsysnqn}"
-	_remove_nvmet_port "${port}"
-	_remove_nvmet_host "${def_hostnqn}"
-
-	losetup -d "${loop_dev}"
-
-	rm -f "${def_file_path}"
+	_nvmet_target_cleanup
 
 	echo "Test complete"
 }
diff --git a/tests/nvme/024 b/tests/nvme/024
index 7a89ddd79fd9..2308b42968e1 100755
--- a/tests/nvme/024
+++ b/tests/nvme/024
@@ -20,16 +20,9 @@ test() {
 
 	_setup_nvmet
 
-	local port
 	local nvmedev
 
-	truncate -s "${nvme_img_size}" "${def_file_path}"
-
-	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}" \
-		"${def_subsys_uuid}"
-	port="$(_create_nvmet_port "${nvme_trtype}")"
-	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
-	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
+	_nvmet_target_setup --blkdev=file
 
 	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
 
@@ -42,12 +35,7 @@ test() {
 	fi
 	_nvme_disconnect_subsys "${def_subsysnqn}" >> "$FULL" 2>&1
 
-	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
-	_remove_nvmet_subsystem "${def_subsysnqn}"
-	_remove_nvmet_port "${port}"
-	_remove_nvmet_host "${def_hostnqn}"
-
-	rm -f "${def_file_path}"
+	_nvmet_target_cleanup
 
 	echo "Test complete"
 }
diff --git a/tests/nvme/025 b/tests/nvme/025
index 90f214eff6c8..b3851d8ceb14 100755
--- a/tests/nvme/025
+++ b/tests/nvme/025
@@ -20,16 +20,9 @@ test() {
 
 	_setup_nvmet
 
-	local port
 	local nvmedev
 
-	truncate -s "${nvme_img_size}" "${def_file_path}"
-
-	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}" \
-		"${def_subsys_uuid}"
-	port="$(_create_nvmet_port "${nvme_trtype}")"
-	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
-	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
+	_nvmet_target_setup --blkdev=file
 
 	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
 
@@ -43,12 +36,7 @@ test() {
 
 	_nvme_disconnect_subsys "${def_subsysnqn}" >> "$FULL" 2>&1
 
-	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
-	_remove_nvmet_subsystem "${def_subsysnqn}"
-	_remove_nvmet_port "${port}"
-	_remove_nvmet_host "${def_hostnqn}"
-
-	rm -f "${def_file_path}"
+	_nvmet_target_cleanup
 
 	echo "Test complete"
 }
diff --git a/tests/nvme/026 b/tests/nvme/026
index ec352acaa489..38acfcc373b4 100755
--- a/tests/nvme/026
+++ b/tests/nvme/026
@@ -20,16 +20,9 @@ test() {
 
 	_setup_nvmet
 
-	local port
 	local nvmedev
 
-	truncate -s "${nvme_img_size}" "${def_file_path}"
-
-	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}" \
-		"${def_subsys_uuid}"
-	port="$(_create_nvmet_port "${nvme_trtype}")"
-	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
-	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
+	_nvmet_target_setup --blkdev=file
 
 	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
 
@@ -43,12 +36,7 @@ test() {
 
 	_nvme_disconnect_subsys "${def_subsysnqn}" >> "$FULL" 2>&1
 
-	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
-	_remove_nvmet_subsystem "${def_subsysnqn}"
-	_remove_nvmet_port "${port}"
-	_remove_nvmet_host "${def_hostnqn}"
-
-	rm -f "${def_file_path}"
+	_nvmet_target_cleanup
 
 	echo "Test complete"
 }
diff --git a/tests/nvme/027 b/tests/nvme/027
index 339f7605a9f5..2d65b3e1a820 100755
--- a/tests/nvme/027
+++ b/tests/nvme/027
@@ -20,16 +20,9 @@ test() {
 
 	_setup_nvmet
 
-	local port
 	local nvmedev
 
-	truncate -s "${nvme_img_size}" "${def_file_path}"
-
-	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}" \
-		"${def_subsys_uuid}"
-	port="$(_create_nvmet_port "${nvme_trtype}")"
-	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
-	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
+	_nvmet_target_setup --blkdev=file
 
 	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
 
@@ -40,14 +33,10 @@ test() {
 	if ! nvme ns-rescan "/dev/${nvmedev}" >> "$FULL" 2>&1; then
 		echo "ERROR: ns-rescan failed"
 	fi
-	_nvme_disconnect_subsys "${def_subsysnqn}" >> "$FULL" 2>&1
 
-	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
-	_remove_nvmet_subsystem "${def_subsysnqn}"
-	_remove_nvmet_port "${port}"
-	_remove_nvmet_host "${def_hostnqn}"
+	_nvme_disconnect_subsys "${def_subsysnqn}" >> "$FULL" 2>&1
 
-	rm -f "${def_file_path}"
+	_nvmet_target_cleanup
 
 	echo "Test complete"
 }
diff --git a/tests/nvme/028 b/tests/nvme/028
index 7f387eb337f6..eec1807884a9 100755
--- a/tests/nvme/028
+++ b/tests/nvme/028
@@ -20,16 +20,9 @@ test() {
 
 	_setup_nvmet
 
-	local port
 	local nvmedev
 
-	truncate -s "${nvme_img_size}" "${def_file_path}"
-
-	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}" \
-		"${def_subsys_uuid}"
-	port="$(_create_nvmet_port "${nvme_trtype}")"
-	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
-	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
+	_nvmet_target_setup --blkdev=file
 
 	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
 
@@ -40,14 +33,10 @@ test() {
 	if ! nvme list-subsys 2>> "$FULL" | grep -q "${nvme_trtype}"; then
 		echo "ERROR: list-subsys"
 	fi
-	_nvme_disconnect_subsys "${def_subsysnqn}" >> "$FULL" 2>&1
 
-	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
-	_remove_nvmet_subsystem "${def_subsysnqn}"
-	_remove_nvmet_port "${port}"
-	_remove_nvmet_host "${def_hostnqn}"
+	_nvme_disconnect_subsys "${def_subsysnqn}" >> "$FULL" 2>&1
 
-	rm -f "${def_file_path}"
+	_nvmet_target_cleanup
 
 	echo "Test complete"
 }
diff --git a/tests/nvme/029 b/tests/nvme/029
index 461e6c6c4454..bbc481437fc8 100755
--- a/tests/nvme/029
+++ b/tests/nvme/029
@@ -53,19 +53,9 @@ test() {
 
 	_setup_nvmet
 
-	local port
 	local nvmedev
-	local loop_dev
 
-	truncate -s "${nvme_img_size}" "${def_file_path}"
-
-	loop_dev="$(losetup -f --show "${def_file_path}")"
-
-	_create_nvmet_subsystem "${def_subsysnqn}" "${loop_dev}" \
-		 "${def_subsys_uuid}"
-	port="$(_create_nvmet_port "${nvme_trtype}")"
-	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
-	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
+	_nvmet_target_setup
 
 	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
 
@@ -83,14 +73,7 @@ test() {
 
 	_nvme_disconnect_subsys "${def_subsysnqn}" >> "$FULL" 2>&1
 
-	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
-	_remove_nvmet_subsystem "${def_subsysnqn}"
-	_remove_nvmet_port "${port}"
-	_remove_nvmet_host "${def_hostnqn}"
-
-	losetup -d "${loop_dev}"
-
-	rm "${def_file_path}"
+	_nvmet_target_cleanup
 
 	echo "Test complete"
 }
diff --git a/tests/nvme/040 b/tests/nvme/040
index ed6df3bbed52..7759bac9b43c 100755
--- a/tests/nvme/040
+++ b/tests/nvme/040
@@ -21,18 +21,10 @@ test() {
 
 	_setup_nvmet
 
-	local port
-	local loop_dev
 	local nvmedev
 	local fio_pid
 
-	truncate -s "${nvme_img_size}" "${def_file_path}"
-	loop_dev="$(losetup -f --show "${def_file_path}")"
-
-	port="$(_create_nvmet_port "${nvme_trtype}")"
-	_create_nvmet_subsystem "${def_subsysnqn}" "${loop_dev}"
-	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
-	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
+	_nvmet_target_setup
 
 	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
 	nvmedev=$(_find_nvme_dev "${def_subsysnqn}")
@@ -54,14 +46,7 @@ test() {
 
 	{ kill "${fio_pid}"; wait; } &> /dev/null
 
-	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
-	_remove_nvmet_subsystem "${def_subsysnqn}"
-	_remove_nvmet_port "${port}"
-	_remove_nvmet_host "${def_hostnqn}"
-
-	losetup -d "${loop_dev}"
-
-	rm -f "${def_file_path}"
+	_nvmet_target_cleanup
 
 	echo "Test complete"
 }
diff --git a/tests/nvme/041 b/tests/nvme/041
index bc84412ccb46..99c24ad4e6a2 100755
--- a/tests/nvme/041
+++ b/tests/nvme/041
@@ -24,7 +24,6 @@ test() {
 
 	_setup_nvmet
 
-	local port
 	local hostkey
 	local ctrldev
 
@@ -34,13 +33,7 @@ test() {
 		return 1
 	fi
 
-	truncate -s "${nvme_img_size}" "${def_file_path}"
-
-	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}" \
-		"${def_subsys_uuid}"
-	port="$(_create_nvmet_port "${nvme_trtype}")"
-	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
-	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}" "${hostkey}"
+	_nvmet_target_setup --blkdev=file --hostkey "${hostkey}"
 
 	# Test unauthenticated connection (should fail)
 	echo "Test unauthenticated connection (should fail)"
@@ -59,14 +52,7 @@ test() {
 
 	_nvme_disconnect_subsys "${def_subsysnqn}"
 
-	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
-	_remove_nvmet_subsystem "${def_subsysnqn}"
-
-	_remove_nvmet_port "${port}"
-
-	_remove_nvmet_host "${def_hostnqn}"
-
-	rm "${def_file_path}"
+	_nvmet_target_cleanup
 
 	echo "Test complete"
 }
diff --git a/tests/nvme/042 b/tests/nvme/042
index 47e1b95ffdc6..f0e196a13ba0 100755
--- a/tests/nvme/042
+++ b/tests/nvme/042
@@ -24,18 +24,12 @@ test() {
 
 	_setup_nvmet
 
-	local port
 	local hmac
 	local key_len
 	local hostkey
 	local ctrldev
 
-	truncate -s "${nvme_img_size}" "${def_file_path}"
-
-	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}"
-	port="$(_create_nvmet_port "${nvme_trtype}")"
-	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
-	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
+	_nvmet_target_setup --blkdev=file
 
 	for hmac in 0 1 2 3; do
 		echo "Testing hmac ${hmac}"
@@ -71,14 +65,7 @@ test() {
 		_nvme_disconnect_subsys "${def_subsysnqn}"
 	done
 
-	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
-	_remove_nvmet_subsystem "${def_subsysnqn}"
-
-	_remove_nvmet_port "${port}"
-
-	_remove_nvmet_host "${def_hostnqn}"
-
-	rm "${def_file_path}"
+	_nvmet_target_cleanup
 
 	echo "Test complete"
 }
diff --git a/tests/nvme/043 b/tests/nvme/043
index 15676f88d556..c95f38e6c71b 100755
--- a/tests/nvme/043
+++ b/tests/nvme/043
@@ -25,7 +25,6 @@ test() {
 
 	_setup_nvmet
 
-	local port
 	local hash
 	local dhgroup
 	local hostkey
@@ -37,12 +36,7 @@ test() {
 		return 1
 	fi
 
-	truncate -s "${nvme_img_size}" "${def_file_path}"
-
-	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}"
-	port="$(_create_nvmet_port "${nvme_trtype}")"
-	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
-	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}" "${hostkey}"
+	_nvmet_target_setup --blkdev=file --hostkey "${hostkey}"
 
 	for hash in "hmac(sha256)" "hmac(sha384)" "hmac(sha512)" ; do
 
@@ -72,14 +66,7 @@ test() {
 		_nvme_disconnect_subsys "${def_subsysnqn}"
 	done
 
-	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
-	_remove_nvmet_subsystem "${def_subsysnqn}"
-
-	_remove_nvmet_port "${port}"
-
-	_remove_nvmet_host "${def_hostnqn}"
-
-	rm "${def_file_path}"
+	_nvmet_target_cleanup
 
 	echo "Test complete"
 }
diff --git a/tests/nvme/044 b/tests/nvme/044
index 9407ac6338c8..f48458a82323 100755
--- a/tests/nvme/044
+++ b/tests/nvme/044
@@ -25,7 +25,6 @@ test() {
 
 	_setup_nvmet
 
-	local port
 	local hostkey
 	local ctrlkey
 	local ctrldev
@@ -42,13 +41,8 @@ test() {
 		return 1
 	fi
 
-	truncate -s "${nvme_img_size}" "${def_file_path}"
-
-	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}"
-	port="$(_create_nvmet_port "${nvme_trtype}")"
-	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
-	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}" \
-			   "${hostkey}" "${ctrlkey}"
+	_nvmet_target_setup --blkdev=file --ctrlkey "${ctrlkey}" \
+			    --hostkey "${hostkey}"
 
 	_set_nvmet_dhgroup "${def_hostnqn}" "ffdhe2048"
 
@@ -95,14 +89,7 @@ test() {
 
 	_nvme_disconnect_subsys "${def_subsysnqn}"
 
-	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
-	_remove_nvmet_subsystem "${def_subsysnqn}"
-
-	_remove_nvmet_port "${port}"
-
-	_remove_nvmet_host "${def_hostnqn}"
-
-	rm "${def_file_path}"
+	_nvmet_target_cleanup
 
 	echo "Test complete"
 }
diff --git a/tests/nvme/045 b/tests/nvme/045
index 396bcdefbcba..902eb26bef8e 100755
--- a/tests/nvme/045
+++ b/tests/nvme/045
@@ -26,7 +26,6 @@ test() {
 
 	_setup_nvmet
 
-	local port
 	local hostkey
 	local new_hostkey
 	local ctrlkey
@@ -46,12 +45,8 @@ test() {
 		return 1
 	fi
 
-	truncate -s "${nvme_img_size}" "${def_file_path}"
-
-	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}"
-	port="$(_create_nvmet_port "${nvme_trtype}")"
-	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
-	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}" "${hostkey}" "${ctrlkey}"
+	_nvmet_target_setup --blkdev=file --ctrlkey "${ctrlkey}" \
+			    --hostkey "${hostkey}"
 
 	_set_nvmet_dhgroup "${def_hostnqn}" "ffdhe2048"
 
@@ -114,14 +109,7 @@ test() {
 
 	_nvme_disconnect_subsys "${def_subsysnqn}"
 
-	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
-	_remove_nvmet_subsystem "${def_subsysnqn}"
-
-	_remove_nvmet_port "${port}"
-
-	_remove_nvmet_host "${def_hostnqn}"
-
-	rm "${def_file_path}"
+	_nvmet_target_cleanup
 
 	echo "Test complete"
 }
diff --git a/tests/nvme/047 b/tests/nvme/047
index 1da24b5638a6..94d7d50f9f98 100755
--- a/tests/nvme/047
+++ b/tests/nvme/047
@@ -22,20 +22,10 @@ test() {
 
 	_setup_nvmet
 
-	local port
 	local nvmedev
-	local loop_dev
 	local rand_io_size
 
-	truncate -s "${nvme_img_size}" "${def_file_path}"
-
-	loop_dev="$(losetup -f --show "${def_file_path}")"
-
-	_create_nvmet_subsystem "${def_subsysnqn}" "${loop_dev}" \
-		"${def_subsys_uuid}"
-	port="$(_create_nvmet_port "${nvme_trtype}")"
-	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
-	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
+	_nvmet_target_setup
 
 	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}" \
 		--nr-write-queues 1 || echo FAIL
@@ -55,14 +45,7 @@ test() {
 
 	_nvme_disconnect_subsys "${def_subsysnqn}" >> "$FULL" 2>&1
 
-	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
-	_remove_nvmet_subsystem "${def_subsysnqn}"
-	_remove_nvmet_port "${port}"
-	_remove_nvmet_host "${def_hostnqn}"
-
-	losetup -d "${loop_dev}"
-
-	rm "${def_file_path}"
+	_nvmet_target_cleanup
 
 	echo "Test complete"
 }
diff --git a/tests/nvme/048 b/tests/nvme/048
index 19234a5b3791..06e1fa6b4f65 100755
--- a/tests/nvme/048
+++ b/tests/nvme/048
@@ -87,16 +87,8 @@ test() {
 
 	local cfs_path="${NVMET_CFS}/subsystems/${def_subsysnqn}"
 	local skipped=false
-	local port
 
-	truncate -s "${nvme_img_size}" "${def_file_path}"
-
-	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}" \
-		"${def_subsys_uuid}"
-	port="$(_create_nvmet_port "${nvme_trtype}")"
-
-	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
-	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
+	_nvmet_target_setup --blkdev=file
 
 	if [[ -f "${cfs_path}/attr_qid_max" ]] ; then
 		_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}" \
@@ -118,12 +110,7 @@ test() {
 		skipped=true
 	fi
 
-	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
-	_remove_nvmet_subsystem "${def_subsysnqn}"
-	_remove_nvmet_port "${port}"
-	_remove_nvmet_host "${def_hostnqn}"
-
-	rm "${def_file_path}"
+	_nvmet_target_cleanup
 
 	if [[ "${skipped}" = true ]] ; then
 		return 1
diff --git a/tests/nvme/rc b/tests/nvme/rc
index fdffc07da34a..261892ed2070 100644
--- a/tests/nvme/rc
+++ b/tests/nvme/rc
@@ -359,6 +359,12 @@ _cleanup_nvmet() {
 	if [[ "${nvme_trtype}" == "rdma" ]]; then
 		stop_soft_rdma
 	fi
+
+	blkdev="$(losetup -l | awk '$6 == "'"${def_file_path}"'" { print $1 }')"
+	for dev in ${blkdev}; do
+		losetup -d "${dev}"
+	done
+	rm -f "${def_file_path}"
 }
 
 _setup_nvmet() {
@@ -818,6 +824,69 @@ _find_nvme_passthru_loop_dev() {
 	echo "/dev/${dev}n${nsid}"
 }
 
+_nvmet_target_setup() {
+	local blkdev_type="device"
+	local blkdev
+	local ctrlkey=""
+	local hostkey=""
+	local port
+
+	while [[ $# -gt 0 ]]; do
+		case $1 in
+			--blkdev)
+				blkdev_type="$2"
+				shift 2
+				;;
+			--ctrlkey)
+				ctrlkey="$2"
+				shift 2
+				;;
+			--hostkey)
+				hostkey="$2"
+				shift 2
+				;;
+			*)
+				shift
+				;;
+		esac
+	done
+
+	truncate -s "${nvme_img_size}" "${def_file_path}"
+	if [[ "${blkdev_type}" == "device" ]]; then
+		blkdev="$(losetup -f --show "${def_file_path}")"
+	else
+		blkdev="${def_file_path}"
+	fi
+
+	_create_nvmet_subsystem "${def_subsysnqn}" "${blkdev}" \
+				"${def_subsys_uuid}"
+	port="$(_create_nvmet_port "${nvme_trtype}")"
+	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
+	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}" \
+			"${hostkey}" "${ctrlkey}"
+}
+
+_nvmet_target_cleanup() {
+	local ports
+	local port
+	local blkdev
+
+	_get_nvmet_ports "${def_subsysnqn}" ports
+
+	for port in "${ports[@]}"; do
+		_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
+		_remove_nvmet_port "${port}"
+	done
+	_remove_nvmet_subsystem "${def_subsysnqn}"
+	_remove_nvmet_host "${def_hostnqn}"
+
+	blkdev="$(losetup -l | awk '$6 == "'"${def_file_path}"'" { print $1 }')"
+	if [[ -n "${blkdev}" ]] ; then
+		losetup -d "${blkdev}"
+	fi
+	rm "${def_file_path}"
+}
+
 _nvmet_passthru_target_setup() {
 	local subsys_name=$1
 	local port
-- 
2.41.0

Re: [PATCH blktests v3 0/3] Introduce nvmet target setup/cleanup helpers

[Chaitanya Kulkarni]

On 8/22/23 01:38, Daniel Wagner wrote:
> Addressed Sagi's feedback and dropped the explicit --blkdev=device argument from
> the _nvmet_target_setup calls, as it is the default.
>
>
>
> original cover letter:
>
> Introduce helpers to setup nvmet targets. This is spin off from the refactoring
> patches and the allowed_host patches [1].
>
> Sagi suggested to record all resources allocated by nvmet_target_setup and then
> later clean them up in nvmet_target_cleanup. I opted to figure out in
> nvmet_target_cleanup what was allocated via the newly introdcuded _get_nvmet_ports
> helper. The reason being, Hannes told me offline that he would like to add ANA
> tests which will add some more ports to the subsystem. I hope with this
> the code is more future proof.
>
> BTW, while looking at this I saw that the passthru code is using the awkward
> return value port when calling nvmet_passthru_target_setup. It seems some
> more refactoring is in order...
>
> [1] https://lore.kernel.org/linux-nvme/5h333eqhtw252sjw6axjewlb5bbb5ze7awekczxe3kie2lnhw6@manyer42khct/
>
>
> changes
>
> v3:
>   - rebased/retested
>   - use the default with _nvmet_target_setup
>
> v2:
>   - drop local subsys variable in passthru tests
>   - do not use port as handle in passthru tests
>   - free port after unregistering from subsys
>   - https://lore.kernel.org/linux-nvme/20230818141537.22332-1-dwagner@suse.de/
>
> v1:
>   - https://lore.kernel.org/linux-nvme/20230818095744.24619-1-dwagner@suse.de/
>
> Daniel Wagner (3):
>    nvme/{033,034,035,036}: use default subsysnqn variable directly
>    nvme/{033,034,035,036,37}: drop port handle between passthru target
>      setup and cleanup
>    nvme: introduce nvmet_target_{setup/cleanup} common code
>
>

I really appreciate you doing this cleanup, it was long due.

Thanks a lot, for this series :-

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

-ck

Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code

[Shinichiro Kawasaki]

On Aug 22, 2023 / 10:38, Daniel Wagner wrote:
> Almost all fabric tests have the identically code for
> setting up and cleaning up the target side. Introduce
> two new helpers.
> 
> Signed-off-by: Daniel Wagner <dwagner@suse.de>

CC+: Bart,

This patch makes shellcheck unhappy:

tests/nvme/003:26:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
tests/nvme/004:26:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
tests/nvme/005:26:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
tests/nvme/006:24:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
tests/nvme/008:25:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
tests/nvme/010:25:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
tests/nvme/012:29:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
tests/nvme/014:28:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
tests/nvme/018:26:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
tests/nvme/019:27:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
tests/nvme/023:25:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]

But I think the warn SC2119 is false-positive and we should suppress it. In the
past, blktests had suppressed it until the recent commit 26664dff17b6 ("Do not
suppress any shellcheck warnings"). I think this commit should be reverted
together with this series.


Daniel, please find two more comments in line.

> ---
>  tests/nvme/003 | 14 +++-------
>  tests/nvme/004 | 21 +++------------
>  tests/nvme/005 | 20 ++-------------
>  tests/nvme/006 | 19 ++------------
>  tests/nvme/007 | 14 ++--------
>  tests/nvme/008 | 21 ++-------------
>  tests/nvme/009 | 16 ++----------
>  tests/nvme/010 | 21 ++-------------
>  tests/nvme/011 | 16 ++----------
>  tests/nvme/012 | 21 ++-------------
>  tests/nvme/013 | 16 ++----------
>  tests/nvme/014 | 21 ++-------------
>  tests/nvme/015 | 16 ++----------
>  tests/nvme/018 | 16 ++----------
>  tests/nvme/019 | 21 ++-------------
>  tests/nvme/020 | 16 ++----------
>  tests/nvme/021 | 16 ++----------
>  tests/nvme/022 | 16 ++----------
>  tests/nvme/023 | 21 ++-------------
>  tests/nvme/024 | 16 ++----------
>  tests/nvme/025 | 16 ++----------
>  tests/nvme/026 | 16 ++----------
>  tests/nvme/027 | 17 +++----------
>  tests/nvme/028 | 17 +++----------
>  tests/nvme/029 | 21 ++-------------
>  tests/nvme/040 | 19 ++------------
>  tests/nvme/041 | 18 ++-----------
>  tests/nvme/042 | 17 ++-----------
>  tests/nvme/043 | 17 ++-----------
>  tests/nvme/044 | 19 +++-----------
>  tests/nvme/045 | 18 +++----------
>  tests/nvme/047 | 21 ++-------------
>  tests/nvme/048 | 17 ++-----------
>  tests/nvme/rc  | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  34 files changed, 141 insertions(+), 519 deletions(-)
> 
> diff --git a/tests/nvme/003 b/tests/nvme/003
> index 71b82ce758a3..b5ea2720100e 100755
> --- a/tests/nvme/003
> +++ b/tests/nvme/003
> @@ -22,15 +22,8 @@ test() {
>  
>  	_setup_nvmet
>  
> -	local loop_dev
> -	local port
>  
> -	port="$(_create_nvmet_port "${nvme_trtype}")"
> -
> -	loop_dev="$(losetup -f)"
> -
> -	_create_nvmet_subsystem "${def_subsysnqn}" "${loop_dev}"
> -	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
> +	_nvmet_target_setup
>  
>  	_nvme_connect_subsys "${nvme_trtype}" nqn.2014-08.org.nvmexpress.discovery
>  
> @@ -46,9 +39,8 @@ test() {
>  	fi
>  
>  	_nvme_disconnect_subsys nqn.2014-08.org.nvmexpress.discovery
> -	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
> -	_remove_nvmet_subsystem "${def_subsysnqn}"
> -	_remove_nvmet_port "${port}"
> +
> +	_nvmet_target_cleanup
>  
>  	echo "Test complete"
>  }
> diff --git a/tests/nvme/004 b/tests/nvme/004
> index 697c758d3059..31af8737857b 100755
> --- a/tests/nvme/004
> +++ b/tests/nvme/004
> @@ -22,19 +22,8 @@ test() {
>  
>  	_setup_nvmet
>  
> -	local port
> -	local loop_dev
>  
> -	port="$(_create_nvmet_port "${nvme_trtype}")"
> -
> -	truncate -s "${nvme_img_size}" "${def_file_path}"
> -
> -	loop_dev="$(losetup -f --show "${def_file_path}")"
> -
> -	_create_nvmet_subsystem "${def_subsysnqn}" "${loop_dev}" \
> -		"${def_subsys_uuid}"
> -	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
> -	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
> +	_nvmet_target_setup
>  
>  	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
>  
> @@ -44,12 +33,8 @@ test() {
>  	cat "/sys/block/${nvmedev}n1/wwid"
>  
>  	_nvme_disconnect_subsys ${def_subsysnqn}
> -	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
> -	_remove_nvmet_subsystem "${def_subsysnqn}"
> -	_remove_nvmet_port "${port}"
> -	_remove_nvmet_host "${def_hostnqn}"
> -	losetup -d "$loop_dev"
> -	rm "${def_file_path}"
> +
> +	_nvmet_target_cleanup
>  
>  	echo "Test complete"
>  }
> diff --git a/tests/nvme/005 b/tests/nvme/005
> index 4ca87ff48016..f9956e960a56 100755
> --- a/tests/nvme/005
> +++ b/tests/nvme/005
> @@ -21,20 +21,9 @@ test() {
>  
>  	_setup_nvmet
>  
> -	local port
> -	local loop_dev
>  	local nvmedev
>  
> -	port="$(_create_nvmet_port "${nvme_trtype}")"
> -
> -	truncate -s "${nvme_img_size}" "${def_file_path}"
> -
> -	loop_dev="$(losetup -f --show "${def_file_path}")"
> -
> -	_create_nvmet_subsystem "${def_subsysnqn}" "${loop_dev}" \
> -		"${def_subsys_uuid}"
> -	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
> -	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
> +	_nvmet_target_setup
>  
>  	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
>  
> @@ -43,13 +32,8 @@ test() {
>  	echo 1 > "/sys/class/nvme/${nvmedev}/reset_controller"
>  
>  	_nvme_disconnect_ctrl "${nvmedev}"
> -	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
> -	_remove_nvmet_port "${port}"
> -	_remove_nvmet_subsystem "${def_subsysnqn}"
> -	_remove_nvmet_host "${def_hostnqn}"
>  
> -	losetup -d "$loop_dev"
> -	rm "${def_file_path}"
> +	_nvmet_target_cleanup
>  
>  	echo "Test complete"
>  }
> diff --git a/tests/nvme/006 b/tests/nvme/006
> index 910204aaeb90..d85f64b702eb 100755
> --- a/tests/nvme/006
> +++ b/tests/nvme/006
> @@ -20,25 +20,10 @@ test() {
>  
>  	_setup_nvmet
>  
> -	local port
> -	local loop_dev
>  
> -	truncate -s "${nvme_img_size}" "${def_file_path}"
> +	_nvmet_target_setup
>  
> -	loop_dev="$(losetup -f --show "${def_file_path}")"
> -
> -	_create_nvmet_subsystem "${def_subsysnqn}" "${loop_dev}" \
> -		"${def_subsys_uuid}"
> -	port="$(_create_nvmet_port "${nvme_trtype}")"
> -	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
> -
> -	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
> -	_remove_nvmet_subsystem "${def_subsysnqn}"
> -	_remove_nvmet_port "${port}"
> -
> -	losetup -d "$loop_dev"
> -
> -	rm "${def_file_path}"
> +	_nvmet_target_cleanup
>  
>  	echo "Test complete"
>  }
> diff --git a/tests/nvme/007 b/tests/nvme/007
> index db00bdcc2d08..feac5060a950 100755
> --- a/tests/nvme/007
> +++ b/tests/nvme/007
> @@ -19,20 +19,10 @@ test() {
>  
>  	_setup_nvmet
>  
> -	local port
>  
> -	truncate -s "${nvme_img_size}" "${def_file_path}"
> +	_nvmet_target_setup --blkdev=file
>  
> -	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}" \
> -		"${def_subsys_uuid}"
> -	port="$(_create_nvmet_port "${nvme_trtype}")"
> -	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
> -
> -	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
> -	_remove_nvmet_subsystem "${def_subsysnqn}"
> -	_remove_nvmet_port "${port}"
> -
> -	rm "${def_file_path}"
> +	_nvmet_target_cleanup
>  
>  	echo "Test complete"
>  }
> diff --git a/tests/nvme/008 b/tests/nvme/008
> index bd5e10fbfb99..f4b45b2f1c11 100755
> --- a/tests/nvme/008
> +++ b/tests/nvme/008
> @@ -20,19 +20,9 @@ test() {
>  
>  	_setup_nvmet
>  
> -	local port
>  	local nvmedev
> -	local loop_dev
>  
> -	truncate -s "${nvme_img_size}" "${def_file_path}"
> -
> -	loop_dev="$(losetup -f --show "${def_file_path}")"
> -
> -	_create_nvmet_subsystem "${def_subsysnqn}" "${loop_dev}" \
> -		"${def_subsys_uuid}"
> -	port="$(_create_nvmet_port "${nvme_trtype}")"
> -	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
> -	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
> +	_nvmet_target_setup
>  
>  	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
>  
> @@ -42,14 +32,7 @@ test() {
>  
>  	_nvme_disconnect_subsys "${def_subsysnqn}"
>  
> -	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
> -	_remove_nvmet_subsystem "${def_subsysnqn}"
> -	_remove_nvmet_port "${port}"
> -	_remove_nvmet_host "${def_hostnqn}"
> -
> -	losetup -d "${loop_dev}"
> -
> -	rm "${def_file_path}"
> +	_nvmet_target_cleanup
>  
>  	echo "Test complete"
>  }
> diff --git a/tests/nvme/009 b/tests/nvme/009
> index c9a4b57ac288..63614c91a96d 100755
> --- a/tests/nvme/009
> +++ b/tests/nvme/009
> @@ -19,16 +19,9 @@ test() {
>  
>  	_setup_nvmet
>  
> -	local port
>  	local nvmedev
>  
> -	truncate -s "${nvme_img_size}" "${def_file_path}"
> -
> -	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}" \
> -		"${def_subsys_uuid}"
> -	port="$(_create_nvmet_port "${nvme_trtype}")"
> -	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
> -	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
> +	_nvmet_target_setup --blkdev=file
>  
>  	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
>  
> @@ -38,12 +31,7 @@ test() {
>  
>  	_nvme_disconnect_subsys "${def_subsysnqn}"
>  
> -	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
> -	_remove_nvmet_subsystem "${def_subsysnqn}"
> -	_remove_nvmet_port "${port}"
> -	_remove_nvmet_host "${def_hostnqn}"
> -
> -	rm "${def_file_path}"
> +	_nvmet_target_cleanup
>  
>  	echo "Test complete"
>  }
> diff --git a/tests/nvme/010 b/tests/nvme/010
> index 19bb7f3fc7a7..e782a9bb06f3 100755
> --- a/tests/nvme/010
> +++ b/tests/nvme/010
> @@ -20,19 +20,9 @@ test() {
>  
>  	_setup_nvmet
>  
> -	local port
>  	local nvmedev
> -	local loop_dev
>  
> -	truncate -s "${nvme_img_size}" "${def_file_path}"
> -
> -	loop_dev="$(losetup -f --show "${def_file_path}")"
> -
> -	_create_nvmet_subsystem "${def_subsysnqn}" "${loop_dev}" \
> -		"${def_subsys_uuid}"
> -	port="$(_create_nvmet_port "${nvme_trtype}")"
> -	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
> -	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
> +	_nvmet_target_setup
>  
>  	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
>  
> @@ -45,14 +35,7 @@ test() {
>  
>  	_nvme_disconnect_subsys "${def_subsysnqn}"
>  
> -	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
> -	_remove_nvmet_subsystem "${def_subsysnqn}"
> -	_remove_nvmet_port "${port}"
> -	_remove_nvmet_host "${def_hostnqn}"
> -
> -	losetup -d "${loop_dev}"
> -
> -	rm "${def_file_path}"
> +	_nvmet_target_cleanup
>  
>  	echo "Test complete"
>  }
> diff --git a/tests/nvme/011 b/tests/nvme/011
> index 0e54c2588bc8..7329e0505f59 100755
> --- a/tests/nvme/011
> +++ b/tests/nvme/011
> @@ -20,16 +20,9 @@ test() {
>  
>  	_setup_nvmet
>  
> -	local port
>  	local nvmedev
>  
> -	truncate -s "${nvme_img_size}" "${def_file_path}"
> -
> -	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}" \
> -		"${def_subsys_uuid}"
> -	port="$(_create_nvmet_port "${nvme_trtype}")"
> -	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
> -	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
> +	_nvmet_target_setup --blkdev=file
>  
>  	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
>  
> @@ -42,12 +35,7 @@ test() {
>  
>  	_nvme_disconnect_subsys "${def_subsysnqn}"
>  
> -	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
> -	_remove_nvmet_subsystem "${def_subsysnqn}"
> -	_remove_nvmet_port "${port}"
> -	_remove_nvmet_host "${def_hostnqn}"
> -
> -	rm "${def_file_path}"
> +	_nvmet_target_cleanup
>  
>  	echo "Test complete"
>  }
> diff --git a/tests/nvme/012 b/tests/nvme/012
> index c6b82c821bf2..6072eed3532a 100755
> --- a/tests/nvme/012
> +++ b/tests/nvme/012
> @@ -24,19 +24,9 @@ test() {
>  
>  	_setup_nvmet
>  
> -	local port
>  	local nvmedev
> -	local loop_dev
>  
> -	truncate -s "${nvme_img_size}" "${def_file_path}"
> -
> -	loop_dev="$(losetup -f --show "${def_file_path}")"
> -
> -	_create_nvmet_subsystem "${def_subsysnqn}" "${loop_dev}" \
> -		"${def_subsys_uuid}"
> -	port="$(_create_nvmet_port "${nvme_trtype}")"
> -	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
> -	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
> +	_nvmet_target_setup
>  
>  	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
>  
> @@ -48,14 +38,7 @@ test() {
>  
>  	_nvme_disconnect_subsys "${def_subsysnqn}"
>  
> -	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
> -	_remove_nvmet_subsystem "${def_subsysnqn}"
> -	_remove_nvmet_port "${port}"
> -	_remove_nvmet_host "${def_hostnqn}"
> -
> -	losetup -d "${loop_dev}"
> -
> -	rm "${def_file_path}"
> +	_nvmet_target_cleanup
>  
>  	echo "Test complete"
>  }
> diff --git a/tests/nvme/013 b/tests/nvme/013
> index 441db7477d75..c9be60675cc4 100755
> --- a/tests/nvme/013
> +++ b/tests/nvme/013
> @@ -23,16 +23,9 @@ test() {
>  
>  	_setup_nvmet
>  
> -	local port
>  	local nvmedev
>  
> -	truncate -s "${nvme_img_size}" "${def_file_path}"
> -
> -	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}" \
> -		"${def_subsys_uuid}"
> -	port="$(_create_nvmet_port "${nvme_trtype}")"
> -	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
> -	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
> +	_nvmet_target_setup --blkdev=file
>  
>  	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
>  
> @@ -44,12 +37,7 @@ test() {
>  
>  	_nvme_disconnect_subsys "${def_subsysnqn}"
>  
> -	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
> -	_remove_nvmet_subsystem "${def_subsysnqn}"
> -	_remove_nvmet_port "${port}"
> -	_remove_nvmet_host "${def_hostnqn}"
> -
> -	rm "${def_file_path}"
> +	_nvmet_target_cleanup
>  
>  	echo "Test complete"
>  }
> diff --git a/tests/nvme/014 b/tests/nvme/014
> index 3656f9399687..d49e8f3cce4d 100755
> --- a/tests/nvme/014
> +++ b/tests/nvme/014
> @@ -20,22 +20,12 @@ test() {
>  
>  	_setup_nvmet
>  
> -	local port
>  	local nvmedev
> -	local loop_dev
>  	local size
>  	local bs
>  	local count
>  
> -	truncate -s "${nvme_img_size}" "${def_file_path}"
> -
> -	loop_dev="$(losetup -f --show "${def_file_path}")"
> -
> -	_create_nvmet_subsystem "${def_subsysnqn}" "${loop_dev}" \
> -		 "${def_subsys_uuid}"
> -	port="$(_create_nvmet_port "${nvme_trtype}")"
> -	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
> -	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
> +	_nvmet_target_setup
>  
>  	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
>  
> @@ -54,14 +44,7 @@ test() {
>  
>  	_nvme_disconnect_subsys "${def_subsysnqn}"
>  
> -	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
> -	_remove_nvmet_subsystem "${def_subsysnqn}"
> -	_remove_nvmet_port "${port}"
> -	_remove_nvmet_host "${def_hostnqn}"
> -
> -	losetup -d "${loop_dev}"
> -
> -	rm "${def_file_path}"
> +	_nvmet_target_cleanup
>  
>  	echo "Test complete"
>  }
> diff --git a/tests/nvme/015 b/tests/nvme/015
> index bc04e39c628c..b418d785ab27 100755
> --- a/tests/nvme/015
> +++ b/tests/nvme/015
> @@ -20,19 +20,12 @@ test() {
>  
>  	_setup_nvmet
>  
> -	local port
>  	local nvmedev
>  	local size
>  	local bs
>  	local count
>  
> -	truncate -s "${nvme_img_size}" "${def_file_path}"
> -
> -	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}" \
> -		 "${def_subsys_uuid}"
> -	port="$(_create_nvmet_port "${nvme_trtype}")"
> -	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
> -	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
> +	_nvmet_target_setup --blkdev=file
>  
>  	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
>  
> @@ -51,12 +44,7 @@ test() {
>  
>  	_nvme_disconnect_subsys "${def_subsysnqn}"
>  
> -	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
> -	_remove_nvmet_subsystem "${def_subsysnqn}"
> -	_remove_nvmet_port "${port}"
> -	_remove_nvmet_host "${def_hostnqn}"
> -
> -	rm "${def_file_path}"
> +	_nvmet_target_cleanup
>  
>  	echo "Test complete"
>  }
> diff --git a/tests/nvme/018 b/tests/nvme/018
> index 68729c3cb070..19e439f3f3e0 100755
> --- a/tests/nvme/018
> +++ b/tests/nvme/018
> @@ -21,16 +21,9 @@ test() {
>  
>  	_setup_nvmet
>  
> -	local port
>  	local nvmedev
>  
> -	truncate -s "${nvme_img_size}" "${def_file_path}"
> -
> -	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}" \
> -		 "${def_subsys_uuid}"
> -	port="$(_create_nvmet_port "${nvme_trtype}")"
> -	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
> -	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
> +	_nvmet_target_setup

I think the line above misses --blkdev=file, doesn't it?

>  
>  	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
>  
> @@ -48,12 +41,7 @@ test() {
>  
>  	_nvme_disconnect_subsys "${def_subsysnqn}"
>  
> -	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
> -	_remove_nvmet_subsystem "${def_subsysnqn}"
> -	_remove_nvmet_port "${port}"
> -	_remove_nvmet_host "${def_hostnqn}"
> -
> -	rm "${def_file_path}"
> +	_nvmet_target_cleanup
>  
>  	echo "Test complete"
>  }
> diff --git a/tests/nvme/019 b/tests/nvme/019
> index 33a25d52e9fd..15e98c40134f 100755
> --- a/tests/nvme/019
> +++ b/tests/nvme/019
> @@ -20,21 +20,11 @@ test() {
>  
>  	_setup_nvmet
>  
> -	local port
>  	local nvmedev
> -	local loop_dev
>  	local nblk_range="10,10,10,10,10,10,10,10,10,10"
>  	local sblk_range="100,200,300,400,500,600,700,800,900,1000"
>  
> -	truncate -s "${nvme_img_size}" "${def_file_path}"
> -
> -	loop_dev="$(losetup -f --show "${def_file_path}")"
> -
> -	_create_nvmet_subsystem "${def_subsysnqn}" "${loop_dev}" \
> -		"${def_subsys_uuid}"
> -	port="$(_create_nvmet_port "${nvme_trtype}")"
> -	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
> -	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
> +	_nvmet_target_setup
>  
>  	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
>  
> @@ -46,14 +36,7 @@ test() {
>  
>  	_nvme_disconnect_subsys "${def_subsysnqn}"
>  
> -	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
> -	_remove_nvmet_subsystem "${def_subsysnqn}"
> -	_remove_nvmet_port "${port}"
> -	_remove_nvmet_host "${def_hostnqn}"
> -
> -	losetup -d "${loop_dev}"
> -
> -	rm -f "${def_file_path}"
> +	_nvmet_target_cleanup
>  
>  	echo "Test complete"
>  }
> diff --git a/tests/nvme/020 b/tests/nvme/020
> index f436cdc8b262..aae40e7131e0 100755
> --- a/tests/nvme/020
> +++ b/tests/nvme/020
> @@ -19,18 +19,11 @@ test() {
>  
>  	_setup_nvmet
>  
> -	local port
>  	local nvmedev
>  	local nblk_range="10,10,10,10,10,10,10,10,10,10"
>  	local sblk_range="100,200,300,400,500,600,700,800,900,1000"
>  
> -	truncate -s "${nvme_img_size}" "${def_file_path}"
> -
> -	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}" \
> -		"${def_subsys_uuid}"
> -	port="$(_create_nvmet_port "${nvme_trtype}")"
> -	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
> -	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
> +	_nvmet_target_setup --blkdev=file
>  
>  	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
>  
> @@ -42,12 +35,7 @@ test() {
>  
>  	_nvme_disconnect_subsys "${def_subsysnqn}"
>  
> -	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
> -	_remove_nvmet_subsystem "${def_subsysnqn}"
> -	_remove_nvmet_port "${port}"
> -	_remove_nvmet_host "${def_hostnqn}"
> -
> -	rm -f "${def_file_path}"
> +	_nvmet_target_cleanup
>  
>  	echo "Test complete"
>  }
> diff --git a/tests/nvme/021 b/tests/nvme/021
> index 5043fe4916be..f9bed1546307 100755
> --- a/tests/nvme/021
> +++ b/tests/nvme/021
> @@ -20,16 +20,9 @@ test() {
>  
>  	_setup_nvmet
>  
> -	local port
>  	local nvmedev
>  
> -	truncate -s "${nvme_img_size}" "${def_file_path}"
> -
> -	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}" \
> -		"${def_subsys_uuid}"
> -	port="$(_create_nvmet_port "${nvme_trtype}")"
> -	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
> -	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
> +	_nvmet_target_setup --blkdev=file
>  
>  	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
>  
> @@ -43,12 +36,7 @@ test() {
>  
>  	_nvme_disconnect_subsys "${def_subsysnqn}" >> "$FULL" 2>&1
>  
> -	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
> -	_remove_nvmet_subsystem "${def_subsysnqn}"
> -	_remove_nvmet_port "${port}"
> -	_remove_nvmet_host "${def_hostnqn}"
> -
> -	rm -f "${def_file_path}"
> +	_nvmet_target_cleanup
>  
>  	echo "Test complete"
>  }
> diff --git a/tests/nvme/022 b/tests/nvme/022
> index 8b6f610c4894..e3e67b0996df 100755
> --- a/tests/nvme/022
> +++ b/tests/nvme/022
> @@ -20,16 +20,9 @@ test() {
>  
>  	_setup_nvmet
>  
> -	local port
>  	local nvmedev
>  
> -	truncate -s "${nvme_img_size}" "${def_file_path}"
> -
> -	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}" \
> -		"${def_subsys_uuid}"
> -	port="$(_create_nvmet_port "${nvme_trtype}")"
> -	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
> -	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
> +	_nvmet_target_setup --blkdev=file
>  
>  	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
>  
> @@ -43,12 +36,7 @@ test() {
>  
>  	_nvme_disconnect_subsys "${def_subsysnqn}" >> "$FULL" 2>&1
>  
> -	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
> -	_remove_nvmet_subsystem "${def_subsysnqn}"
> -	_remove_nvmet_port "${port}"
> -	_remove_nvmet_host "${def_hostnqn}"
> -
> -	rm -f "${def_file_path}"
> +	_nvmet_target_cleanup
>  
>  	echo "Test complete"
>  }
> diff --git a/tests/nvme/023 b/tests/nvme/023
> index 90af0338e81f..c8d1e4619822 100755
> --- a/tests/nvme/023
> +++ b/tests/nvme/023
> @@ -20,19 +20,9 @@ test() {
>  
>  	_setup_nvmet
>  
> -	local port
>  	local nvmedev
> -	local loop_dev
>  
> -	truncate -s "${nvme_img_size}" "${def_file_path}"
> -
> -	loop_dev="$(losetup -f --show "${def_file_path}")"
> -
> -	_create_nvmet_subsystem "${def_subsysnqn}" "${loop_dev}" \
> -		"${def_subsys_uuid}"
> -	port="$(_create_nvmet_port "${nvme_trtype}")"
> -	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
> -	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
> +	_nvmet_target_setup
>  
>  	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
>  
> @@ -46,14 +36,7 @@ test() {
>  
>  	_nvme_disconnect_subsys "${def_subsysnqn}" >> "$FULL" 2>&1
>  
> -	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
> -	_remove_nvmet_subsystem "${def_subsysnqn}"
> -	_remove_nvmet_port "${port}"
> -	_remove_nvmet_host "${def_hostnqn}"
> -
> -	losetup -d "${loop_dev}"
> -
> -	rm -f "${def_file_path}"
> +	_nvmet_target_cleanup
>  
>  	echo "Test complete"
>  }
> diff --git a/tests/nvme/024 b/tests/nvme/024
> index 7a89ddd79fd9..2308b42968e1 100755
> --- a/tests/nvme/024
> +++ b/tests/nvme/024
> @@ -20,16 +20,9 @@ test() {
>  
>  	_setup_nvmet
>  
> -	local port
>  	local nvmedev
>  
> -	truncate -s "${nvme_img_size}" "${def_file_path}"
> -
> -	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}" \
> -		"${def_subsys_uuid}"
> -	port="$(_create_nvmet_port "${nvme_trtype}")"
> -	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
> -	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
> +	_nvmet_target_setup --blkdev=file
>  
>  	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
>  
> @@ -42,12 +35,7 @@ test() {
>  	fi
>  	_nvme_disconnect_subsys "${def_subsysnqn}" >> "$FULL" 2>&1
>  
> -	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
> -	_remove_nvmet_subsystem "${def_subsysnqn}"
> -	_remove_nvmet_port "${port}"
> -	_remove_nvmet_host "${def_hostnqn}"
> -
> -	rm -f "${def_file_path}"
> +	_nvmet_target_cleanup
>  
>  	echo "Test complete"
>  }
> diff --git a/tests/nvme/025 b/tests/nvme/025
> index 90f214eff6c8..b3851d8ceb14 100755
> --- a/tests/nvme/025
> +++ b/tests/nvme/025
> @@ -20,16 +20,9 @@ test() {
>  
>  	_setup_nvmet
>  
> -	local port
>  	local nvmedev
>  
> -	truncate -s "${nvme_img_size}" "${def_file_path}"
> -
> -	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}" \
> -		"${def_subsys_uuid}"
> -	port="$(_create_nvmet_port "${nvme_trtype}")"
> -	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
> -	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
> +	_nvmet_target_setup --blkdev=file
>  
>  	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
>  
> @@ -43,12 +36,7 @@ test() {
>  
>  	_nvme_disconnect_subsys "${def_subsysnqn}" >> "$FULL" 2>&1
>  
> -	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
> -	_remove_nvmet_subsystem "${def_subsysnqn}"
> -	_remove_nvmet_port "${port}"
> -	_remove_nvmet_host "${def_hostnqn}"
> -
> -	rm -f "${def_file_path}"
> +	_nvmet_target_cleanup
>  
>  	echo "Test complete"
>  }
> diff --git a/tests/nvme/026 b/tests/nvme/026
> index ec352acaa489..38acfcc373b4 100755
> --- a/tests/nvme/026
> +++ b/tests/nvme/026
> @@ -20,16 +20,9 @@ test() {
>  
>  	_setup_nvmet
>  
> -	local port
>  	local nvmedev
>  
> -	truncate -s "${nvme_img_size}" "${def_file_path}"
> -
> -	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}" \
> -		"${def_subsys_uuid}"
> -	port="$(_create_nvmet_port "${nvme_trtype}")"
> -	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
> -	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
> +	_nvmet_target_setup --blkdev=file
>  
>  	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
>  
> @@ -43,12 +36,7 @@ test() {
>  
>  	_nvme_disconnect_subsys "${def_subsysnqn}" >> "$FULL" 2>&1
>  
> -	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
> -	_remove_nvmet_subsystem "${def_subsysnqn}"
> -	_remove_nvmet_port "${port}"
> -	_remove_nvmet_host "${def_hostnqn}"
> -
> -	rm -f "${def_file_path}"
> +	_nvmet_target_cleanup
>  
>  	echo "Test complete"
>  }
> diff --git a/tests/nvme/027 b/tests/nvme/027
> index 339f7605a9f5..2d65b3e1a820 100755
> --- a/tests/nvme/027
> +++ b/tests/nvme/027
> @@ -20,16 +20,9 @@ test() {
>  
>  	_setup_nvmet
>  
> -	local port
>  	local nvmedev
>  
> -	truncate -s "${nvme_img_size}" "${def_file_path}"
> -
> -	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}" \
> -		"${def_subsys_uuid}"
> -	port="$(_create_nvmet_port "${nvme_trtype}")"
> -	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
> -	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
> +	_nvmet_target_setup --blkdev=file
>  
>  	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
>  
> @@ -40,14 +33,10 @@ test() {
>  	if ! nvme ns-rescan "/dev/${nvmedev}" >> "$FULL" 2>&1; then
>  		echo "ERROR: ns-rescan failed"
>  	fi
> -	_nvme_disconnect_subsys "${def_subsysnqn}" >> "$FULL" 2>&1
>  
> -	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
> -	_remove_nvmet_subsystem "${def_subsysnqn}"
> -	_remove_nvmet_port "${port}"
> -	_remove_nvmet_host "${def_hostnqn}"
> +	_nvme_disconnect_subsys "${def_subsysnqn}" >> "$FULL" 2>&1
>  
> -	rm -f "${def_file_path}"
> +	_nvmet_target_cleanup
>  
>  	echo "Test complete"
>  }
> diff --git a/tests/nvme/028 b/tests/nvme/028
> index 7f387eb337f6..eec1807884a9 100755
> --- a/tests/nvme/028
> +++ b/tests/nvme/028
> @@ -20,16 +20,9 @@ test() {
>  
>  	_setup_nvmet
>  
> -	local port
>  	local nvmedev
>  
> -	truncate -s "${nvme_img_size}" "${def_file_path}"
> -
> -	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}" \
> -		"${def_subsys_uuid}"
> -	port="$(_create_nvmet_port "${nvme_trtype}")"
> -	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
> -	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
> +	_nvmet_target_setup --blkdev=file
>  
>  	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
>  
> @@ -40,14 +33,10 @@ test() {
>  	if ! nvme list-subsys 2>> "$FULL" | grep -q "${nvme_trtype}"; then
>  		echo "ERROR: list-subsys"
>  	fi
> -	_nvme_disconnect_subsys "${def_subsysnqn}" >> "$FULL" 2>&1
>  
> -	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
> -	_remove_nvmet_subsystem "${def_subsysnqn}"
> -	_remove_nvmet_port "${port}"
> -	_remove_nvmet_host "${def_hostnqn}"
> +	_nvme_disconnect_subsys "${def_subsysnqn}" >> "$FULL" 2>&1
>  
> -	rm -f "${def_file_path}"
> +	_nvmet_target_cleanup
>  
>  	echo "Test complete"
>  }
> diff --git a/tests/nvme/029 b/tests/nvme/029
> index 461e6c6c4454..bbc481437fc8 100755
> --- a/tests/nvme/029
> +++ b/tests/nvme/029
> @@ -53,19 +53,9 @@ test() {
>  
>  	_setup_nvmet
>  
> -	local port
>  	local nvmedev
> -	local loop_dev
>  
> -	truncate -s "${nvme_img_size}" "${def_file_path}"
> -
> -	loop_dev="$(losetup -f --show "${def_file_path}")"
> -
> -	_create_nvmet_subsystem "${def_subsysnqn}" "${loop_dev}" \
> -		 "${def_subsys_uuid}"
> -	port="$(_create_nvmet_port "${nvme_trtype}")"
> -	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
> -	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
> +	_nvmet_target_setup
>  
>  	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
>  
> @@ -83,14 +73,7 @@ test() {
>  
>  	_nvme_disconnect_subsys "${def_subsysnqn}" >> "$FULL" 2>&1
>  
> -	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
> -	_remove_nvmet_subsystem "${def_subsysnqn}"
> -	_remove_nvmet_port "${port}"
> -	_remove_nvmet_host "${def_hostnqn}"
> -
> -	losetup -d "${loop_dev}"
> -
> -	rm "${def_file_path}"
> +	_nvmet_target_cleanup
>  
>  	echo "Test complete"
>  }
> diff --git a/tests/nvme/040 b/tests/nvme/040
> index ed6df3bbed52..7759bac9b43c 100755
> --- a/tests/nvme/040
> +++ b/tests/nvme/040
> @@ -21,18 +21,10 @@ test() {
>  
>  	_setup_nvmet
>  
> -	local port
> -	local loop_dev
>  	local nvmedev
>  	local fio_pid
>  
> -	truncate -s "${nvme_img_size}" "${def_file_path}"
> -	loop_dev="$(losetup -f --show "${def_file_path}")"
> -
> -	port="$(_create_nvmet_port "${nvme_trtype}")"
> -	_create_nvmet_subsystem "${def_subsysnqn}" "${loop_dev}"
> -	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
> -	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
> +	_nvmet_target_setup
>  
>  	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
>  	nvmedev=$(_find_nvme_dev "${def_subsysnqn}")
> @@ -54,14 +46,7 @@ test() {
>  
>  	{ kill "${fio_pid}"; wait; } &> /dev/null
>  
> -	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
> -	_remove_nvmet_subsystem "${def_subsysnqn}"
> -	_remove_nvmet_port "${port}"
> -	_remove_nvmet_host "${def_hostnqn}"
> -
> -	losetup -d "${loop_dev}"
> -
> -	rm -f "${def_file_path}"
> +	_nvmet_target_cleanup
>  
>  	echo "Test complete"
>  }
> diff --git a/tests/nvme/041 b/tests/nvme/041
> index bc84412ccb46..99c24ad4e6a2 100755
> --- a/tests/nvme/041
> +++ b/tests/nvme/041
> @@ -24,7 +24,6 @@ test() {
>  
>  	_setup_nvmet
>  
> -	local port
>  	local hostkey
>  	local ctrldev
>  
> @@ -34,13 +33,7 @@ test() {
>  		return 1
>  	fi
>  
> -	truncate -s "${nvme_img_size}" "${def_file_path}"
> -
> -	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}" \
> -		"${def_subsys_uuid}"
> -	port="$(_create_nvmet_port "${nvme_trtype}")"
> -	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
> -	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}" "${hostkey}"
> +	_nvmet_target_setup --blkdev=file --hostkey "${hostkey}"
>  
>  	# Test unauthenticated connection (should fail)
>  	echo "Test unauthenticated connection (should fail)"
> @@ -59,14 +52,7 @@ test() {
>  
>  	_nvme_disconnect_subsys "${def_subsysnqn}"
>  
> -	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
> -	_remove_nvmet_subsystem "${def_subsysnqn}"
> -
> -	_remove_nvmet_port "${port}"
> -
> -	_remove_nvmet_host "${def_hostnqn}"
> -
> -	rm "${def_file_path}"
> +	_nvmet_target_cleanup
>  
>  	echo "Test complete"
>  }
> diff --git a/tests/nvme/042 b/tests/nvme/042
> index 47e1b95ffdc6..f0e196a13ba0 100755
> --- a/tests/nvme/042
> +++ b/tests/nvme/042
> @@ -24,18 +24,12 @@ test() {
>  
>  	_setup_nvmet
>  
> -	local port
>  	local hmac
>  	local key_len
>  	local hostkey
>  	local ctrldev
>  
> -	truncate -s "${nvme_img_size}" "${def_file_path}"
> -
> -	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}"
> -	port="$(_create_nvmet_port "${nvme_trtype}")"
> -	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
> -	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
> +	_nvmet_target_setup --blkdev=file
>  
>  	for hmac in 0 1 2 3; do
>  		echo "Testing hmac ${hmac}"
> @@ -71,14 +65,7 @@ test() {
>  		_nvme_disconnect_subsys "${def_subsysnqn}"
>  	done
>  
> -	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
> -	_remove_nvmet_subsystem "${def_subsysnqn}"
> -
> -	_remove_nvmet_port "${port}"
> -
> -	_remove_nvmet_host "${def_hostnqn}"
> -
> -	rm "${def_file_path}"
> +	_nvmet_target_cleanup
>  
>  	echo "Test complete"
>  }
> diff --git a/tests/nvme/043 b/tests/nvme/043
> index 15676f88d556..c95f38e6c71b 100755
> --- a/tests/nvme/043
> +++ b/tests/nvme/043
> @@ -25,7 +25,6 @@ test() {
>  
>  	_setup_nvmet
>  
> -	local port
>  	local hash
>  	local dhgroup
>  	local hostkey
> @@ -37,12 +36,7 @@ test() {
>  		return 1
>  	fi
>  
> -	truncate -s "${nvme_img_size}" "${def_file_path}"
> -
> -	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}"
> -	port="$(_create_nvmet_port "${nvme_trtype}")"
> -	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
> -	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}" "${hostkey}"
> +	_nvmet_target_setup --blkdev=file --hostkey "${hostkey}"
>  
>  	for hash in "hmac(sha256)" "hmac(sha384)" "hmac(sha512)" ; do
>  
> @@ -72,14 +66,7 @@ test() {
>  		_nvme_disconnect_subsys "${def_subsysnqn}"
>  	done
>  
> -	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
> -	_remove_nvmet_subsystem "${def_subsysnqn}"
> -
> -	_remove_nvmet_port "${port}"
> -
> -	_remove_nvmet_host "${def_hostnqn}"
> -
> -	rm "${def_file_path}"
> +	_nvmet_target_cleanup
>  
>  	echo "Test complete"
>  }
> diff --git a/tests/nvme/044 b/tests/nvme/044
> index 9407ac6338c8..f48458a82323 100755
> --- a/tests/nvme/044
> +++ b/tests/nvme/044
> @@ -25,7 +25,6 @@ test() {
>  
>  	_setup_nvmet
>  
> -	local port
>  	local hostkey
>  	local ctrlkey
>  	local ctrldev
> @@ -42,13 +41,8 @@ test() {
>  		return 1
>  	fi
>  
> -	truncate -s "${nvme_img_size}" "${def_file_path}"
> -
> -	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}"
> -	port="$(_create_nvmet_port "${nvme_trtype}")"
> -	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
> -	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}" \
> -			   "${hostkey}" "${ctrlkey}"
> +	_nvmet_target_setup --blkdev=file --ctrlkey "${ctrlkey}" \
> +			    --hostkey "${hostkey}"
>  
>  	_set_nvmet_dhgroup "${def_hostnqn}" "ffdhe2048"
>  
> @@ -95,14 +89,7 @@ test() {
>  
>  	_nvme_disconnect_subsys "${def_subsysnqn}"
>  
> -	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
> -	_remove_nvmet_subsystem "${def_subsysnqn}"
> -
> -	_remove_nvmet_port "${port}"
> -
> -	_remove_nvmet_host "${def_hostnqn}"
> -
> -	rm "${def_file_path}"
> +	_nvmet_target_cleanup
>  
>  	echo "Test complete"
>  }
> diff --git a/tests/nvme/045 b/tests/nvme/045
> index 396bcdefbcba..902eb26bef8e 100755
> --- a/tests/nvme/045
> +++ b/tests/nvme/045
> @@ -26,7 +26,6 @@ test() {
>  
>  	_setup_nvmet
>  
> -	local port
>  	local hostkey
>  	local new_hostkey
>  	local ctrlkey
> @@ -46,12 +45,8 @@ test() {
>  		return 1
>  	fi
>  
> -	truncate -s "${nvme_img_size}" "${def_file_path}"
> -
> -	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}"
> -	port="$(_create_nvmet_port "${nvme_trtype}")"
> -	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
> -	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}" "${hostkey}" "${ctrlkey}"
> +	_nvmet_target_setup --blkdev=file --ctrlkey "${ctrlkey}" \
> +			    --hostkey "${hostkey}"
>  
>  	_set_nvmet_dhgroup "${def_hostnqn}" "ffdhe2048"
>  
> @@ -114,14 +109,7 @@ test() {
>  
>  	_nvme_disconnect_subsys "${def_subsysnqn}"
>  
> -	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
> -	_remove_nvmet_subsystem "${def_subsysnqn}"
> -
> -	_remove_nvmet_port "${port}"
> -
> -	_remove_nvmet_host "${def_hostnqn}"
> -
> -	rm "${def_file_path}"
> +	_nvmet_target_cleanup
>  
>  	echo "Test complete"
>  }
> diff --git a/tests/nvme/047 b/tests/nvme/047
> index 1da24b5638a6..94d7d50f9f98 100755
> --- a/tests/nvme/047
> +++ b/tests/nvme/047
> @@ -22,20 +22,10 @@ test() {
>  
>  	_setup_nvmet
>  
> -	local port
>  	local nvmedev
> -	local loop_dev
>  	local rand_io_size
>  
> -	truncate -s "${nvme_img_size}" "${def_file_path}"
> -
> -	loop_dev="$(losetup -f --show "${def_file_path}")"
> -
> -	_create_nvmet_subsystem "${def_subsysnqn}" "${loop_dev}" \
> -		"${def_subsys_uuid}"
> -	port="$(_create_nvmet_port "${nvme_trtype}")"
> -	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
> -	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
> +	_nvmet_target_setup
>  
>  	_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}" \
>  		--nr-write-queues 1 || echo FAIL
> @@ -55,14 +45,7 @@ test() {
>  
>  	_nvme_disconnect_subsys "${def_subsysnqn}" >> "$FULL" 2>&1
>  
> -	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
> -	_remove_nvmet_subsystem "${def_subsysnqn}"
> -	_remove_nvmet_port "${port}"
> -	_remove_nvmet_host "${def_hostnqn}"
> -
> -	losetup -d "${loop_dev}"
> -
> -	rm "${def_file_path}"
> +	_nvmet_target_cleanup
>  
>  	echo "Test complete"
>  }
> diff --git a/tests/nvme/048 b/tests/nvme/048
> index 19234a5b3791..06e1fa6b4f65 100755
> --- a/tests/nvme/048
> +++ b/tests/nvme/048
> @@ -87,16 +87,8 @@ test() {
>  
>  	local cfs_path="${NVMET_CFS}/subsystems/${def_subsysnqn}"
>  	local skipped=false
> -	local port
>  
> -	truncate -s "${nvme_img_size}" "${def_file_path}"
> -
> -	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}" \
> -		"${def_subsys_uuid}"
> -	port="$(_create_nvmet_port "${nvme_trtype}")"
> -
> -	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
> -	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
> +	_nvmet_target_setup --blkdev=file
>  
>  	if [[ -f "${cfs_path}/attr_qid_max" ]] ; then
>  		_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}" \
> @@ -118,12 +110,7 @@ test() {
>  		skipped=true
>  	fi
>  
> -	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
> -	_remove_nvmet_subsystem "${def_subsysnqn}"
> -	_remove_nvmet_port "${port}"
> -	_remove_nvmet_host "${def_hostnqn}"
> -
> -	rm "${def_file_path}"
> +	_nvmet_target_cleanup
>  
>  	if [[ "${skipped}" = true ]] ; then
>  		return 1
> diff --git a/tests/nvme/rc b/tests/nvme/rc
> index fdffc07da34a..261892ed2070 100644
> --- a/tests/nvme/rc
> +++ b/tests/nvme/rc
> @@ -359,6 +359,12 @@ _cleanup_nvmet() {
>  	if [[ "${nvme_trtype}" == "rdma" ]]; then
>  		stop_soft_rdma
>  	fi
> +
> +	blkdev="$(losetup -l | awk '$6 == "'"${def_file_path}"'" { print $1 }')"
> +	for dev in ${blkdev}; do
> +		losetup -d "${dev}"
> +	done
> +	rm -f "${def_file_path}"
>  }
>  
>  _setup_nvmet() {

The added lines above are same as those in _nvmet_target_cleanup(). So, those
liens are executed twice. Is this what you intend? At least, I think it's the
better to factor out the lines into another helper.

> @@ -818,6 +824,69 @@ _find_nvme_passthru_loop_dev() {
>  	echo "/dev/${dev}n${nsid}"
>  }
>  
> +_nvmet_target_setup() {
> +	local blkdev_type="device"
> +	local blkdev
> +	local ctrlkey=""
> +	local hostkey=""
> +	local port
> +
> +	while [[ $# -gt 0 ]]; do
> +		case $1 in
> +			--blkdev)
> +				blkdev_type="$2"
> +				shift 2
> +				;;
> +			--ctrlkey)
> +				ctrlkey="$2"
> +				shift 2
> +				;;
> +			--hostkey)
> +				hostkey="$2"
> +				shift 2
> +				;;
> +			*)
> +				shift
> +				;;
> +		esac
> +	done
> +
> +	truncate -s "${nvme_img_size}" "${def_file_path}"
> +	if [[ "${blkdev_type}" == "device" ]]; then
> +		blkdev="$(losetup -f --show "${def_file_path}")"
> +	else
> +		blkdev="${def_file_path}"
> +	fi
> +
> +	_create_nvmet_subsystem "${def_subsysnqn}" "${blkdev}" \
> +				"${def_subsys_uuid}"
> +	port="$(_create_nvmet_port "${nvme_trtype}")"
> +	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
> +	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}" \
> +			"${hostkey}" "${ctrlkey}"
> +}
> +
> +_nvmet_target_cleanup() {
> +	local ports
> +	local port
> +	local blkdev
> +
> +	_get_nvmet_ports "${def_subsysnqn}" ports
> +
> +	for port in "${ports[@]}"; do
> +		_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
> +		_remove_nvmet_port "${port}"
> +	done
> +	_remove_nvmet_subsystem "${def_subsysnqn}"
> +	_remove_nvmet_host "${def_hostnqn}"
> +
> +	blkdev="$(losetup -l | awk '$6 == "'"${def_file_path}"'" { print $1 }')"
> +	if [[ -n "${blkdev}" ]] ; then
> +		losetup -d "${blkdev}"
> +	fi
> +	rm "${def_file_path}"
> +}
> +
>  _nvmet_passthru_target_setup() {
>  	local subsys_name=$1
>  	local port
> -- 
> 2.41.0
> 

Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code

[Bart Van Assche]

On 8/23/23 20:09, Shinichiro Kawasaki wrote:
> CC+: Bart,
> 
> This patch makes shellcheck unhappy:
> 
> tests/nvme/003:26:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
> tests/nvme/004:26:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
> tests/nvme/005:26:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
> tests/nvme/006:24:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
> tests/nvme/008:25:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
> tests/nvme/010:25:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
> tests/nvme/012:29:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
> tests/nvme/014:28:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
> tests/nvme/018:26:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
> tests/nvme/019:27:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
> tests/nvme/023:25:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
> 
> But I think the warn SC2119 is false-positive and we should suppress it. In the
> past, blktests had suppressed it until the recent commit 26664dff17b6 ("Do not
> suppress any shellcheck warnings"). I think this commit should be reverted
> together with this series.
Please do not revert commit 26664dff17b6 because it produces useful
warnings. Do you agree that the above warnings are easy to suppress,
e.g. by changing "_nvmet_target_setup" into
"_nvmet_target_setup ignored_argument"?

Thanks,

Bart.

Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code

[Shinichiro Kawasaki]

On Aug 24, 2023 / 07:36, Bart Van Assche wrote:
> On 8/23/23 20:09, Shinichiro Kawasaki wrote:
> > CC+: Bart,
> > 
> > This patch makes shellcheck unhappy:
> > 
> > tests/nvme/003:26:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
> > tests/nvme/004:26:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
> > tests/nvme/005:26:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
> > tests/nvme/006:24:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
> > tests/nvme/008:25:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
> > tests/nvme/010:25:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
> > tests/nvme/012:29:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
> > tests/nvme/014:28:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
> > tests/nvme/018:26:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
> > tests/nvme/019:27:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
> > tests/nvme/023:25:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
> > 
> > But I think the warn SC2119 is false-positive and we should suppress it. In the
> > past, blktests had suppressed it until the recent commit 26664dff17b6 ("Do not
> > suppress any shellcheck warnings"). I think this commit should be reverted
> > together with this series.
> Please do not revert commit 26664dff17b6 because it produces useful
> warnings.

Hmm, I see... SC2119 is false-positive for this patch, but it sometimes useful
functions which takes "$@" as arguments.

> Do you agree that the above warnings are easy to suppress,
> e.g. by changing "_nvmet_target_setup" into
> "_nvmet_target_setup ignored_argument"?

This works, but a bit ugly. Another idea is to make one of the optional
arguments mandatory, a positional argument. I think the option --device_type
worth making mandatory and explicit, like,

    _nvmet_target_setup device
    _nvmet_target_setup file

This will make it easier for me to review which test case uses which type.
(This might be against Sagi's comment, though.)

Daniel, what do you think?

Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code

[Daniel Wagner]

On Thu, Aug 24, 2023 at 07:36:12AM -0700, Bart Van Assche wrote:
> On 8/23/23 20:09, Shinichiro Kawasaki wrote:
> > CC+: Bart,
> > 
> > This patch makes shellcheck unhappy:
> > 
> > tests/nvme/003:26:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
> > tests/nvme/004:26:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
> > tests/nvme/005:26:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
> > tests/nvme/006:24:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
> > tests/nvme/008:25:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
> > tests/nvme/010:25:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
> > tests/nvme/012:29:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
> > tests/nvme/014:28:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
> > tests/nvme/018:26:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
> > tests/nvme/019:27:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
> > tests/nvme/023:25:2: note: Use _nvmet_target_setup "$@" if function's $1 should mean script's $1. [SC2119]
> > 
> > But I think the warn SC2119 is false-positive and we should suppress it. In the
> > past, blktests had suppressed it until the recent commit 26664dff17b6 ("Do not
> > suppress any shellcheck warnings"). I think this commit should be reverted
> > together with this series.
> Please do not revert commit 26664dff17b6 because it produces useful
> warnings. Do you agree that the above warnings are easy to suppress,
> e.g. by changing "_nvmet_target_setup" into
> "_nvmet_target_setup ignored_argument"?

Well, these warnings could be address by adding back '--blkdev=device',
but I just dropped them on Sagi's request.

So what is it going to be? Ignoring SC2119 or adding and default
argument? I personally would rather add SC2119 because there a lot of
more callers which hand in default arguments which I would like to
remove anyway. Working around ShellCheck current limitation seems wrong
to me.

Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code

[Daniel Wagner]

On Fri, Aug 25, 2023 at 12:53:26AM +0000, Shinichiro Kawasaki wrote:
> This works, but a bit ugly. Another idea is to make one of the optional
> arguments mandatory, a positional argument. I think the option --device_type
> worth making mandatory and explicit, like,
> 
>     _nvmet_target_setup device
>     _nvmet_target_setup file

Possible but as I said in the other mail, we have a lot more of default
arguments to functions which I would like to drop too.

> This will make it easier for me to review which test case uses which type.
> (This might be against Sagi's comment, though.)
> 
> Daniel, what do you think?

I don't think it is good strategy to change blktests just to make
ShellCheck happy, when we know it is broken. It rather have SC2119
ignored until ShellCheck is fixed.

Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code

[Shinichiro Kawasaki]

On Aug 25, 2023 / 08:40, Daniel Wagner wrote:
> On Fri, Aug 25, 2023 at 12:53:26AM +0000, Shinichiro Kawasaki wrote:
> > This works, but a bit ugly. Another idea is to make one of the optional
> > arguments mandatory, a positional argument. I think the option --device_type
> > worth making mandatory and explicit, like,
> > 
> >     _nvmet_target_setup device
> >     _nvmet_target_setup file
> 
> Possible but as I said in the other mail, we have a lot more of default
> arguments to functions which I would like to drop too.
> 
> > This will make it easier for me to review which test case uses which type.
> > (This might be against Sagi's comment, though.)
> > 
> > Daniel, what do you think?
> 
> I don't think it is good strategy to change blktests just to make
> ShellCheck happy, when we know it is broken. It rather have SC2119
> ignored until ShellCheck is fixed.

Thanks for the comments.

IMO, SC2119 is not broken. SC2119 (and its companion SC2120) assumes that bash
functions do not have optional arguments. If any functions which refer arguments
are called without arguments, it complains. With the assumption, SC2119 is
useful to detect missing arguments of function calls. (I guess Bart thinks this
is useful.)

However, when we implement argument parsers in bash functions so that the
arguments can be optional, the assumption for the SC2119 is wrong. Then SC2119
reports are useless. Until recently, blktests had few functions with such
optional arguments, such as _init_null_blk or _init_scsi_debug. But most of
calls to those functions had some arguments, and it was rare to call them
without any argument. So SC2119 reports were easy to suppress and not a pain.

    I doubt Shellcheck can be improved and detect if functions have the optional
    argument parsers...

Recently, you actively cleans up tests/nvme/* (which is great!), and introduced
argument parsers in test/nvme/rc. The first one is _nvme_connect_subsys, and the
second one is this _nvme_target_setup. It looks for me this is a bash coding
style change in blktests, from "don't use optional arguments often" to "use
optional arguments aggressively". If we apply this change, we should suppress
SC2119. If we keep the old coding style, we should keep on enabling SC2119. What
I see here is the style difference between you and Bart.

Now I'm tempted to disable SC2119, and to go with the new coding style...

If I have any misunderstanding, or if anyone has more comments on this, please
let me know.

Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code

[Daniel Wagner]

On Fri, Aug 25, 2023 at 07:34:21AM +0000, Shinichiro Kawasaki wrote:
> IMO, SC2119 is not broken. SC2119 (and its companion SC2120) assumes that bash
> functions do not have optional arguments. If any functions which refer arguments
> are called without arguments, it complains. With the assumption, SC2119 is
> useful to detect missing arguments of function calls. (I guess Bart thinks this
> is useful.)

I wanted to say that the implementation of SC2119 is broken, not the
SC2119/SC2120 itself. Sorry for the confusion.

> However, when we implement argument parsers in bash functions so that the
> arguments can be optional, the assumption for the SC2119 is wrong. Then SC2119
> reports are useless. Until recently, blktests had few functions with such
> optional arguments, such as _init_null_blk or _init_scsi_debug. But most of
> calls to those functions had some arguments, and it was rare to call them
> without any argument. So SC2119 reports were easy to suppress and not a pain.
>
>     I doubt Shellcheck can be improved and detect if functions have the optional
>     argument parsers...

No idea. But we are not alone with this problem:

https://github.com/koalaman/shellcheck/issues/2511

> Recently, you actively cleans up tests/nvme/* (which is great!), and introduced
> argument parsers in test/nvme/rc. The first one is _nvme_connect_subsys, and the
> second one is this _nvme_target_setup. It looks for me this is a bash coding
> style change in blktests, from "don't use optional arguments often" to "use
> optional arguments aggressively".

Yes, it's a bit excessive to hand in all possible arguments all the
time. Especially it makes it even hard to review if only value changes
but 6 default values are passed to the setup function.

> If we apply this change, we should suppress
> SC2119. If we keep the old coding style, we should keep on enabling SC2119. What
> I see here is the style difference between you and Bart.
>
> Now I'm tempted to disable SC2119, and to go with the new coding style...
> 
> If I have any misunderstanding, or if anyone has more comments on this, please
> let me know.

All good from my side.

Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code

[Bart Van Assche]

On 8/25/23 00:34, Shinichiro Kawasaki wrote:
> Recently, you actively cleans up tests/nvme/* (which is great!), and introduced
> argument parsers in test/nvme/rc. The first one is _nvme_connect_subsys, and the
> second one is this _nvme_target_setup. It looks for me this is a bash coding
> style change in blktests, from "don't use optional arguments often" to "use
> optional arguments aggressively". If we apply this change, we should suppress
> SC2119. If we keep the old coding style, we should keep on enabling SC2119. What
> I see here is the style difference between you and Bart.
> 
> Now I'm tempted to disable SC2119, and to go with the new coding style...
> 
> If I have any misunderstanding, or if anyone has more comments on this, please
> let me know.

I don't like the "new style". What is so hard about typing "$@" to pass all function
arguments to _nvmet_target_setup()? Leaving out "$@" makes it much harder than
necessary to figure out the intent of the code author - not passing any arguments
or passing all caller arguments implicitly.

Bart.

Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code

[Daniel Wagner]

On Fri, Aug 25, 2023 at 06:45:25AM -0700, Bart Van Assche wrote:
> On 8/25/23 00:34, Shinichiro Kawasaki wrote:
> > Recently, you actively cleans up tests/nvme/* (which is great!), and introduced
> > argument parsers in test/nvme/rc. The first one is _nvme_connect_subsys, and the
> > second one is this _nvme_target_setup. It looks for me this is a bash coding
> > style change in blktests, from "don't use optional arguments often" to "use
> > optional arguments aggressively". If we apply this change, we should suppress
> > SC2119. If we keep the old coding style, we should keep on enabling SC2119. What
> > I see here is the style difference between you and Bart.
> > 
> > Now I'm tempted to disable SC2119, and to go with the new coding style...
> > 
> > If I have any misunderstanding, or if anyone has more comments on this, please
> > let me know.
> 
> I don't like the "new style". What is so hard about typing "$@" to pass all function
> arguments to _nvmet_target_setup()? Leaving out "$@" makes it much harder than
> necessary to figure out the intent of the code author - not passing any arguments
> or passing all caller arguments implicitly.

Because "$@" is just not correct. Also by using defaults we really see
where the test is special.

Let's look at this here:

 _create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}"

Both arguments are default values and could just be left out. It makes
reading the code way simpler,

 _create_nvmet_subsystem

Another example, if setup a default target

 _nvmet_target_setup

and if we want to enable the auth code:

 _nvmet_target_setup --ctrlkey "${ctrlkey}" --hostkey "${hostkey}"

and that's all. You can easily see what's is different from the default
values.

The "old" style is expecting that the caller gets the number of
arguments and position correct:

 _create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}" "${hostkey}" "${ctrlkey}"

And this isn't always the case. I already fixed a couple of bugs where
the test got the order wrong.

Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code

[Bart Van Assche]

On 8/25/23 07:26, Daniel Wagner wrote:
> On Fri, Aug 25, 2023 at 06:45:25AM -0700, Bart Van Assche wrote:
>> I don't like the "new style". What is so hard about typing "$@" to pass all function
>> arguments to _nvmet_target_setup()? Leaving out "$@" makes it much harder than
>> necessary to figure out the intent of the code author - not passing any arguments
>> or passing all caller arguments implicitly.
> 
> Because "$@" is just not correct.

Why not?

Thanks,

Bart.

Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code

[Shinichiro Kawasaki]

On Aug 25, 2023 / 09:46, Bart Van Assche wrote:
> On 8/25/23 07:26, Daniel Wagner wrote:
> > On Fri, Aug 25, 2023 at 06:45:25AM -0700, Bart Van Assche wrote:
> > > I don't like the "new style". What is so hard about typing "$@" to pass all function
> > > arguments to _nvmet_target_setup()? Leaving out "$@" makes it much harder than
> > > necessary to figure out the intent of the code author - not passing any arguments
> > > or passing all caller arguments implicitly.
> > 
> > Because "$@" is just not correct.
> 
> Why not?

Bart, let me confirm. Do you suggest

    test() {

        _nvmet_target_setup "$@"

instead of this?

    test() {

        _nvmet_target_setup

If so, it looks weird since "$@" in test() is not the parameters passed to
_nvmet_target_setup(). Anyway, I tried the change with test/nvme/003, and
observed the shellcheck warning disappears. Then, it will work so long as "$@"
is empty in the context of _nvmet_target_setup() caller. Otherwise, it will not
work. For me, your original suggestion to add "ignored_agument" looks better
than "$@". (or in short, "noarg" or something)

Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code

[Shinichiro Kawasaki]

On Aug 22, 2023 / 10:38, Daniel Wagner wrote:
[...]
> diff --git a/tests/nvme/015 b/tests/nvme/015
> index bc04e39c628c..b418d785ab27 100755
> --- a/tests/nvme/015
> +++ b/tests/nvme/015
> @@ -20,19 +20,12 @@ test() {
>  
>  	_setup_nvmet
>  
> -	local port
>  	local nvmedev
>  	local size
>  	local bs
>  	local count
>  
> -	truncate -s "${nvme_img_size}" "${def_file_path}"
> -
> -	_create_nvmet_subsystem "${def_subsysnqn}" "${def_file_path}" \
> -		 "${def_subsys_uuid}"
> -	port="$(_create_nvmet_port "${nvme_trtype}")"
> -	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
> -	_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
> +	_nvmet_target_setup --blkdev=file

I looked back and found that the option above looks wrong. I think it should be
"--blkdev file" instead of "--blkedev=file".

Same for other _nvmet_target_setup calls with the option.

Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code

[Bart Van Assche]

On 8/22/23 01:38, Daniel Wagner wrote:
> +	while [[ $# -gt 0 ]]; do
> +		case $1 in
> +			--blkdev)
> +				blkdev_type="$2"
> +				shift 2
> +				;;
> +			--ctrlkey)
> +				ctrlkey="$2"
> +				shift 2
> +				;;
> +			--hostkey)
> +				hostkey="$2"
> +				shift 2
> +				;;
> +			*)
> +				shift
> +				;;
> +		esac
> +	done

So all arguments that are not recognized are ignored? That will
make debugging typo's harder than necessary. Shouldn't this function
complain if an unrecognized argument is encountered?

Thanks,

Bart.

Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code

[Bart Van Assche]

On 8/27/23 21:13, Shinichiro Kawasaki wrote:
> For me, your original suggestion to add "ignored_agument" looks better
> than "$@". (or in short, "noarg" or something)

It is not clear to me what the intention is of the _nvmet_target_setup
calls without arguments. Is the intention to pass all arguments that have
been passed to the caller or is the intention not to pass any arguments?
In the latter case I think it would be wrong to suppress SC2119 because
there really is a problem in this case. How about passing -- as argument
if the intention is not to pass any arguments? It is a well established
convention for shell commands and shell functions to ignore the double
hyphen if it is encountered in the argument list.

Thanks,

Bart.

Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code

[Daniel Wagner]

On Mon, Aug 28, 2023 at 08:14:31AM -0700, Bart Van Assche wrote:
> On 8/27/23 21:13, Shinichiro Kawasaki wrote:
> > For me, your original suggestion to add "ignored_agument" looks better
> > than "$@". (or in short, "noarg" or something)
> 
> It is not clear to me what the intention is of the _nvmet_target_setup
> calls without arguments.

Create a target. That's it. It is really not that complicated.

> Is the intention to pass all arguments that have
> been passed to the caller or is the intention not to pass any
> arguments?

If there are no arguments, the indent is not to pass any arguments.

> In the latter case I think it would be wrong to suppress SC2119 because
> there really is a problem in this case.

IMO, SC2119 is not helping at all. What does it prevent? It doesn't even
understand how many arguments are supposed to be passed into a function.
The few error cases it catches are very limitted.

> How about passing -- as argument
> if the intention is not to pass any arguments? It is a well established
> convention for shell commands and shell functions to ignore the double
> hyphen if it is encountered in the argument list.

I am against adding code just to make ShellCheck happy. If there is
another way achieve this I am all ear.

Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code

[Daniel Wagner]

On Mon, Aug 28, 2023 at 08:14:09AM -0700, Bart Van Assche wrote:
> On 8/22/23 01:38, Daniel Wagner wrote:
> > +	while [[ $# -gt 0 ]]; do
> > +		case $1 in
> > +			--blkdev)
> > +				blkdev_type="$2"
> > +				shift 2
> > +				;;
> > +			--ctrlkey)
> > +				ctrlkey="$2"
> > +				shift 2
> > +				;;
> > +			--hostkey)
> > +				hostkey="$2"
> > +				shift 2
> > +				;;
> > +			*)
> > +				shift
> > +				;;
> > +		esac
> > +	done
> 
> So all arguments that are not recognized are ignored? That will
> make debugging typo's harder than necessary. Shouldn't this function
> complain if an unrecognized argument is encountered?

Sure, I'll add a warning.

Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code

[Bart Van Assche]

On 8/28/23 10:02, Daniel Wagner wrote:
> I am against adding code just to make ShellCheck happy.
Hi Daniel,

That's not what my concern is about. My concern is about keeping
the blktests source code maintainable and easy to read. My opinion
is that the ability of bash to pass arguments from caller to callee
implicitly (a) hurts readability, (b) is error prone and (c) hurts
maintainability. This is why I think that this feature should not
be used and hence that disabling SC2119 would be really wrong.

Regarding (a), I think this long e-mail thread is more than enough
evidence that it is not clear what the intention is of the
_nvmet_target_setup calls without arguments - not pass any arguments
or pass the argument list of the caller. Regarding (c): if any
_nvmet_target_setup calls would be added in a function that accepts
arguments, how is _nvmet_target_setup() expected to process arguments
of which it doesn't know how to interpret these?

Hence my proposal to change the _nvmet_target_setup calls with no
arguments into "_nvmet_target_setup --" and also to ignore the double
hyphen argument inside _nvmet_target_setup().

Thanks,

Bart.

Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code

[Shinichiro Kawasaki]

Thanks Bart, I'm getting more understanding of your view. Still I have an
unclear point.

On Aug 28, 2023 / 11:55, Bart Van Assche wrote:
> On 8/28/23 10:02, Daniel Wagner wrote:
> > I am against adding code just to make ShellCheck happy.
> Hi Daniel,
> 
> That's not what my concern is about. My concern is about keeping
> the blktests source code maintainable and easy to read. My opinion
> is that the ability of bash to pass arguments from caller to callee
> implicitly (a) hurts readability, (b) is error prone and (c) hurts
> maintainability. This is why I think that this feature should not
> be used and hence that disabling SC2119 would be really wrong.
> 
> Regarding (a), I think this long e-mail thread is more than enough
> evidence that it is not clear what the intention is of the
> _nvmet_target_setup calls without arguments - not pass any arguments
> or pass the argument list of the caller.

This is the unclear point for me. Does bash really pass the arguments list of
the caller to the callee when functions are called without arguments?

Looking back the commit 852996fea4f1, you explained that bash does, and I
agreed. But now in my environment bash doesn't. I tried the script below in my
environment, and see nothing printed.

  funcA() { echo "$1" ; }
  funcB() { funcA; }
  funcB foo

Then the arguments of funcB is not passed to funcA. How does it run in your
environment?

Also, I checked bash document [*] and it says:

  "When a function is executed, the arguments to the function become the
  positional parameters during its execution".

It does not clearly state the case there is no argument, but I think this means
that the positional parameters in the function becomes a new, empty array. At
least, the document does not say that the positional parameters of caller is
passed to callee when there is no argument.

[*] https://www.gnu.org/software/bash/manual/html_node/Shell-Functions.html

So I think we can safely assume that bash doesn't pass the argument list of
caller to callee when functions are called without arguments.

> Regarding (c): if any
> _nvmet_target_setup calls would be added in a function that accepts
> arguments, how is _nvmet_target_setup() expected to process arguments
> of which it doesn't know how to interpret these?
> 
> Hence my proposal to change the _nvmet_target_setup calls with no
> arguments into "_nvmet_target_setup --" and also to ignore the double
> hyphen argument inside _nvmet_target_setup().

Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code

[Bart Van Assche]

On 8/28/23 19:11, Shinichiro Kawasaki wrote:
> This is the unclear point for me. Does bash really pass the arguments list of
> the caller to the callee when functions are called without arguments?
> 
> Looking back the commit 852996fea4f1, you explained that bash does, and I
> agreed. But now in my environment bash doesn't. I tried the script below in my
> environment, and see nothing printed.
> 
>    funcA() { echo "$1" ; }
>    funcB() { funcA; }
>    funcB foo
> 
> Then the arguments of funcB is not passed to funcA. How does it run in your
> environment?

I see the same result that you see. It seems that I misinterpret the 
text produced by shellcheck if it reports warning SC2119. After having 
reread https://github.com/koalaman/shellcheck/wiki/SC2119, I'm OK with
suppressing warning SC2119 because that warning doesn't seem useful to
me.

Thanks,

Bart.

Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code

[Shinichiro Kawasaki]

On Aug 29, 2023 / 06:35, Bart Van Assche wrote:
> On 8/28/23 19:11, Shinichiro Kawasaki wrote:
> > This is the unclear point for me. Does bash really pass the arguments list of
> > the caller to the callee when functions are called without arguments?
> > 
> > Looking back the commit 852996fea4f1, you explained that bash does, and I
> > agreed. But now in my environment bash doesn't. I tried the script below in my
> > environment, and see nothing printed.
> > 
> >    funcA() { echo "$1" ; }
> >    funcB() { funcA; }
> >    funcB foo
> > 
> > Then the arguments of funcB is not passed to funcA. How does it run in your
> > environment?
> 
> I see the same result that you see. It seems that I misinterpret the text
> produced by shellcheck if it reports warning SC2119. After having reread
> https://github.com/koalaman/shellcheck/wiki/SC2119, I'm OK with
> suppressing warning SC2119 because that warning doesn't seem useful to
> me.

Thank you for confirmation. It's good that we clarified this confusing point :)
I will revert the 26664dff17b6 ("Do not suppress any shellcheck warnings") to
suppress SC2119.

Later on, I'll create a clean-up-patch for SC2119 which will revert relevant
commits 852996fea4f1 and 45b203cce8b (partially for the latter).

Daniel, let's go ahead with current approach: allow calling _nvmet_target_setup
without arguments.

Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code

[Shinichiro Kawasaki]

On Aug 30, 2023 / 01:19, Shinichiro Kawasaki wrote:
[...]
> Thank you for confirmation. It's good that we clarified this confusing point :)
> I will revert the 26664dff17b6 ("Do not suppress any shellcheck warnings") to
> suppress SC2119.

FYI, the commit 3d1c0fe2677d did the revert.

> 
> Later on, I'll create a clean-up-patch for SC2119 which will revert relevant
> commits 852996fea4f1 and 45b203cce8b (partially for the latter).

Also, I've applied the commit 2045e8d3df86 for the clean-up above. It should be
all good now.