* [PATCH blktests v3 0/3] Introduce nvmet target setup/cleanup helpers
@ 2023-08-22 8:38 Daniel Wagner
2023-08-22 8:38 ` [PATCH blktests v3 1/3] nvme/{033,034,035,036}: use default subsysnqn variable directly Daniel Wagner
` (3 more replies)
0 siblings, 4 replies; 26+ messages in thread
From: Daniel Wagner @ 2023-08-22 8:38 UTC (permalink / raw)
To: linux-nvme
Cc: linux-kernel, linux-block, Chaitanya Kulkarni,
Shin'ichiro Kawasaki, Hannes Reinecke, Sagi Grimberg,
Jason Gunthorpe, 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
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCH blktests v3 1/3] nvme/{033,034,035,036}: use default subsysnqn variable directly 2023-08-22 8:38 [PATCH blktests v3 0/3] Introduce nvmet target setup/cleanup helpers Daniel Wagner @ 2023-08-22 8:38 ` Daniel Wagner 2023-08-22 8:38 ` [PATCH blktests v3 2/3] nvme/{033,034,035,036,37}: drop port handle between passthru target setup and cleanup Daniel Wagner ` (2 subsequent siblings) 3 siblings, 0 replies; 26+ messages in thread From: Daniel Wagner @ 2023-08-22 8:38 UTC (permalink / raw) To: linux-nvme Cc: linux-kernel, linux-block, Chaitanya Kulkarni, Shin'ichiro Kawasaki, Hannes Reinecke, Sagi Grimberg, Jason Gunthorpe, 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH blktests v3 2/3] nvme/{033,034,035,036,37}: drop port handle between passthru target setup and cleanup 2023-08-22 8:38 [PATCH blktests v3 0/3] Introduce nvmet target setup/cleanup helpers Daniel Wagner 2023-08-22 8:38 ` [PATCH blktests v3 1/3] nvme/{033,034,035,036}: use default subsysnqn variable directly Daniel Wagner @ 2023-08-22 8:38 ` Daniel Wagner 2023-08-22 8:38 ` [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code Daniel Wagner 2023-08-22 17:54 ` [PATCH blktests v3 0/3] Introduce nvmet target setup/cleanup helpers Chaitanya Kulkarni 3 siblings, 0 replies; 26+ messages in thread From: Daniel Wagner @ 2023-08-22 8:38 UTC (permalink / raw) To: linux-nvme Cc: linux-kernel, linux-block, Chaitanya Kulkarni, Shin'ichiro Kawasaki, Hannes Reinecke, Sagi Grimberg, Jason Gunthorpe, 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code 2023-08-22 8:38 [PATCH blktests v3 0/3] Introduce nvmet target setup/cleanup helpers Daniel Wagner 2023-08-22 8:38 ` [PATCH blktests v3 1/3] nvme/{033,034,035,036}: use default subsysnqn variable directly Daniel Wagner 2023-08-22 8:38 ` [PATCH blktests v3 2/3] nvme/{033,034,035,036,37}: drop port handle between passthru target setup and cleanup Daniel Wagner @ 2023-08-22 8:38 ` Daniel Wagner 2023-08-24 3:09 ` Shinichiro Kawasaki ` (2 more replies) 2023-08-22 17:54 ` [PATCH blktests v3 0/3] Introduce nvmet target setup/cleanup helpers Chaitanya Kulkarni 3 siblings, 3 replies; 26+ messages in thread From: Daniel Wagner @ 2023-08-22 8:38 UTC (permalink / raw) To: linux-nvme Cc: linux-kernel, linux-block, Chaitanya Kulkarni, Shin'ichiro Kawasaki, Hannes Reinecke, Sagi Grimberg, Jason Gunthorpe, 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code 2023-08-22 8:38 ` [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code Daniel Wagner @ 2023-08-24 3:09 ` Shinichiro Kawasaki 2023-08-24 14:36 ` Bart Van Assche 2023-08-28 4:17 ` Shinichiro Kawasaki 2023-08-28 15:14 ` Bart Van Assche 2 siblings, 1 reply; 26+ messages in thread From: Shinichiro Kawasaki @ 2023-08-24 3:09 UTC (permalink / raw) To: Daniel Wagner Cc: linux-nvme, linux-kernel, linux-block, Chaitanya Kulkarni, Hannes Reinecke, Sagi Grimberg, Jason Gunthorpe, Bart Van Assche 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 > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code 2023-08-24 3:09 ` Shinichiro Kawasaki @ 2023-08-24 14:36 ` Bart Van Assche 2023-08-25 0:53 ` Shinichiro Kawasaki 2023-08-25 6:24 ` Daniel Wagner 0 siblings, 2 replies; 26+ messages in thread From: Bart Van Assche @ 2023-08-24 14:36 UTC (permalink / raw) To: Shinichiro Kawasaki, Daniel Wagner Cc: linux-nvme, linux-kernel, linux-block, Chaitanya Kulkarni, Hannes Reinecke, Sagi Grimberg, Jason Gunthorpe 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code 2023-08-24 14:36 ` Bart Van Assche @ 2023-08-25 0:53 ` Shinichiro Kawasaki 2023-08-25 6:40 ` Daniel Wagner 2023-08-25 6:24 ` Daniel Wagner 1 sibling, 1 reply; 26+ messages in thread From: Shinichiro Kawasaki @ 2023-08-25 0:53 UTC (permalink / raw) To: Bart Van Assche Cc: Daniel Wagner, linux-nvme, linux-kernel, linux-block, Chaitanya Kulkarni, Hannes Reinecke, Sagi Grimberg, Jason Gunthorpe 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? ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code 2023-08-25 0:53 ` Shinichiro Kawasaki @ 2023-08-25 6:40 ` Daniel Wagner 2023-08-25 7:34 ` Shinichiro Kawasaki 0 siblings, 1 reply; 26+ messages in thread From: Daniel Wagner @ 2023-08-25 6:40 UTC (permalink / raw) To: Shinichiro Kawasaki Cc: Bart Van Assche, linux-nvme, linux-kernel, linux-block, Chaitanya Kulkarni, Hannes Reinecke, Sagi Grimberg, Jason Gunthorpe 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code 2023-08-25 6:40 ` Daniel Wagner @ 2023-08-25 7:34 ` Shinichiro Kawasaki 2023-08-25 11:29 ` Daniel Wagner 2023-08-25 13:45 ` Bart Van Assche 0 siblings, 2 replies; 26+ messages in thread From: Shinichiro Kawasaki @ 2023-08-25 7:34 UTC (permalink / raw) To: Daniel Wagner Cc: Bart Van Assche, linux-nvme, linux-kernel, linux-block, Chaitanya Kulkarni, Hannes Reinecke, Sagi Grimberg, Jason Gunthorpe 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code 2023-08-25 7:34 ` Shinichiro Kawasaki @ 2023-08-25 11:29 ` Daniel Wagner 2023-08-25 13:45 ` Bart Van Assche 1 sibling, 0 replies; 26+ messages in thread From: Daniel Wagner @ 2023-08-25 11:29 UTC (permalink / raw) To: Shinichiro Kawasaki Cc: Bart Van Assche, linux-nvme, linux-kernel, linux-block, Chaitanya Kulkarni, Hannes Reinecke, Sagi Grimberg, Jason Gunthorpe 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code 2023-08-25 7:34 ` Shinichiro Kawasaki 2023-08-25 11:29 ` Daniel Wagner @ 2023-08-25 13:45 ` Bart Van Assche 2023-08-25 14:26 ` Daniel Wagner 1 sibling, 1 reply; 26+ messages in thread From: Bart Van Assche @ 2023-08-25 13:45 UTC (permalink / raw) To: Shinichiro Kawasaki, Daniel Wagner Cc: linux-nvme, linux-kernel, linux-block, Chaitanya Kulkarni, Hannes Reinecke, Sagi Grimberg, Jason Gunthorpe 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code 2023-08-25 13:45 ` Bart Van Assche @ 2023-08-25 14:26 ` Daniel Wagner 2023-08-25 16:46 ` Bart Van Assche 0 siblings, 1 reply; 26+ messages in thread From: Daniel Wagner @ 2023-08-25 14:26 UTC (permalink / raw) To: Bart Van Assche Cc: Shinichiro Kawasaki, linux-nvme, linux-kernel, linux-block, Chaitanya Kulkarni, Hannes Reinecke, Sagi Grimberg, Jason Gunthorpe 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code 2023-08-25 14:26 ` Daniel Wagner @ 2023-08-25 16:46 ` Bart Van Assche 2023-08-28 4:13 ` Shinichiro Kawasaki 0 siblings, 1 reply; 26+ messages in thread From: Bart Van Assche @ 2023-08-25 16:46 UTC (permalink / raw) To: Daniel Wagner Cc: Shinichiro Kawasaki, linux-nvme, linux-kernel, linux-block, Chaitanya Kulkarni, Hannes Reinecke, Sagi Grimberg, Jason Gunthorpe 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code 2023-08-25 16:46 ` Bart Van Assche @ 2023-08-28 4:13 ` Shinichiro Kawasaki 2023-08-28 15:14 ` Bart Van Assche 0 siblings, 1 reply; 26+ messages in thread From: Shinichiro Kawasaki @ 2023-08-28 4:13 UTC (permalink / raw) To: Bart Van Assche Cc: Daniel Wagner, linux-nvme, linux-kernel, linux-block, Chaitanya Kulkarni, Hannes Reinecke, Sagi Grimberg, Jason Gunthorpe 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) ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code 2023-08-28 4:13 ` Shinichiro Kawasaki @ 2023-08-28 15:14 ` Bart Van Assche 2023-08-28 17:02 ` Daniel Wagner 0 siblings, 1 reply; 26+ messages in thread From: Bart Van Assche @ 2023-08-28 15:14 UTC (permalink / raw) To: Shinichiro Kawasaki Cc: Daniel Wagner, linux-nvme, linux-kernel, linux-block, Chaitanya Kulkarni, Hannes Reinecke, Sagi Grimberg, Jason Gunthorpe 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code 2023-08-28 15:14 ` Bart Van Assche @ 2023-08-28 17:02 ` Daniel Wagner 2023-08-28 18:55 ` Bart Van Assche 0 siblings, 1 reply; 26+ messages in thread From: Daniel Wagner @ 2023-08-28 17:02 UTC (permalink / raw) To: Bart Van Assche Cc: Shinichiro Kawasaki, linux-nvme, linux-kernel, linux-block, Chaitanya Kulkarni, Hannes Reinecke, Sagi Grimberg, Jason Gunthorpe 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code 2023-08-28 17:02 ` Daniel Wagner @ 2023-08-28 18:55 ` Bart Van Assche 2023-08-29 2:11 ` Shinichiro Kawasaki 0 siblings, 1 reply; 26+ messages in thread From: Bart Van Assche @ 2023-08-28 18:55 UTC (permalink / raw) To: Daniel Wagner Cc: Shinichiro Kawasaki, linux-nvme, linux-kernel, linux-block, Chaitanya Kulkarni, Hannes Reinecke, Sagi Grimberg, Jason Gunthorpe 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code 2023-08-28 18:55 ` Bart Van Assche @ 2023-08-29 2:11 ` Shinichiro Kawasaki 2023-08-29 13:35 ` Bart Van Assche 0 siblings, 1 reply; 26+ messages in thread From: Shinichiro Kawasaki @ 2023-08-29 2:11 UTC (permalink / raw) To: Bart Van Assche Cc: Daniel Wagner, linux-nvme, linux-kernel, linux-block, Chaitanya Kulkarni, Hannes Reinecke, Sagi Grimberg, Jason Gunthorpe 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(). ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code 2023-08-29 2:11 ` Shinichiro Kawasaki @ 2023-08-29 13:35 ` Bart Van Assche 2023-08-30 1:19 ` Shinichiro Kawasaki 0 siblings, 1 reply; 26+ messages in thread From: Bart Van Assche @ 2023-08-29 13:35 UTC (permalink / raw) To: Shinichiro Kawasaki Cc: Daniel Wagner, linux-nvme, linux-kernel, linux-block, Chaitanya Kulkarni, Hannes Reinecke, Sagi Grimberg, Jason Gunthorpe 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code 2023-08-29 13:35 ` Bart Van Assche @ 2023-08-30 1:19 ` Shinichiro Kawasaki 2023-09-01 0:33 ` Shinichiro Kawasaki 0 siblings, 1 reply; 26+ messages in thread From: Shinichiro Kawasaki @ 2023-08-30 1:19 UTC (permalink / raw) To: Bart Van Assche Cc: Daniel Wagner, linux-nvme, linux-kernel, linux-block, Chaitanya Kulkarni, Hannes Reinecke, Sagi Grimberg, Jason Gunthorpe 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code 2023-08-30 1:19 ` Shinichiro Kawasaki @ 2023-09-01 0:33 ` Shinichiro Kawasaki 0 siblings, 0 replies; 26+ messages in thread From: Shinichiro Kawasaki @ 2023-09-01 0:33 UTC (permalink / raw) To: Bart Van Assche Cc: Daniel Wagner, linux-nvme, linux-kernel, linux-block, Chaitanya Kulkarni, Hannes Reinecke, Sagi Grimberg, Jason Gunthorpe 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code 2023-08-24 14:36 ` Bart Van Assche 2023-08-25 0:53 ` Shinichiro Kawasaki @ 2023-08-25 6:24 ` Daniel Wagner 1 sibling, 0 replies; 26+ messages in thread From: Daniel Wagner @ 2023-08-25 6:24 UTC (permalink / raw) To: Bart Van Assche Cc: Shinichiro Kawasaki, linux-nvme, linux-kernel, linux-block, Chaitanya Kulkarni, Hannes Reinecke, Sagi Grimberg, Jason Gunthorpe 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code 2023-08-22 8:38 ` [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code Daniel Wagner 2023-08-24 3:09 ` Shinichiro Kawasaki @ 2023-08-28 4:17 ` Shinichiro Kawasaki 2023-08-28 15:14 ` Bart Van Assche 2 siblings, 0 replies; 26+ messages in thread From: Shinichiro Kawasaki @ 2023-08-28 4:17 UTC (permalink / raw) To: Daniel Wagner Cc: linux-nvme, linux-kernel, linux-block, Chaitanya Kulkarni, Hannes Reinecke, Sagi Grimberg, Jason Gunthorpe 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code 2023-08-22 8:38 ` [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code Daniel Wagner 2023-08-24 3:09 ` Shinichiro Kawasaki 2023-08-28 4:17 ` Shinichiro Kawasaki @ 2023-08-28 15:14 ` Bart Van Assche 2023-08-28 17:03 ` Daniel Wagner 2 siblings, 1 reply; 26+ messages in thread From: Bart Van Assche @ 2023-08-28 15:14 UTC (permalink / raw) To: Daniel Wagner, linux-nvme Cc: linux-kernel, linux-block, Chaitanya Kulkarni, Shin'ichiro Kawasaki, Hannes Reinecke, Sagi Grimberg, Jason Gunthorpe 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code 2023-08-28 15:14 ` Bart Van Assche @ 2023-08-28 17:03 ` Daniel Wagner 0 siblings, 0 replies; 26+ messages in thread From: Daniel Wagner @ 2023-08-28 17:03 UTC (permalink / raw) To: Bart Van Assche Cc: linux-nvme, linux-kernel, linux-block, Chaitanya Kulkarni, Shin'ichiro Kawasaki, Hannes Reinecke, Sagi Grimberg, Jason Gunthorpe 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH blktests v3 0/3] Introduce nvmet target setup/cleanup helpers 2023-08-22 8:38 [PATCH blktests v3 0/3] Introduce nvmet target setup/cleanup helpers Daniel Wagner ` (2 preceding siblings ...) 2023-08-22 8:38 ` [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code Daniel Wagner @ 2023-08-22 17:54 ` Chaitanya Kulkarni 3 siblings, 0 replies; 26+ messages in thread From: Chaitanya Kulkarni @ 2023-08-22 17:54 UTC (permalink / raw) To: Daniel Wagner, linux-nvme Cc: linux-kernel, linux-block, Chaitanya Kulkarni, Shin'ichiro Kawasaki, Hannes Reinecke, Sagi Grimberg, Jason Gunthorpe 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2023-09-01 0:34 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-22 8:38 [PATCH blktests v3 0/3] Introduce nvmet target setup/cleanup helpers Daniel Wagner
2023-08-22 8:38 ` [PATCH blktests v3 1/3] nvme/{033,034,035,036}: use default subsysnqn variable directly Daniel Wagner
2023-08-22 8:38 ` [PATCH blktests v3 2/3] nvme/{033,034,035,036,37}: drop port handle between passthru target setup and cleanup Daniel Wagner
2023-08-22 8:38 ` [PATCH blktests v3 3/3] nvme: introduce nvmet_target_{setup/cleanup} common code Daniel Wagner
2023-08-24 3:09 ` Shinichiro Kawasaki
2023-08-24 14:36 ` Bart Van Assche
2023-08-25 0:53 ` Shinichiro Kawasaki
2023-08-25 6:40 ` Daniel Wagner
2023-08-25 7:34 ` Shinichiro Kawasaki
2023-08-25 11:29 ` Daniel Wagner
2023-08-25 13:45 ` Bart Van Assche
2023-08-25 14:26 ` Daniel Wagner
2023-08-25 16:46 ` Bart Van Assche
2023-08-28 4:13 ` Shinichiro Kawasaki
2023-08-28 15:14 ` Bart Van Assche
2023-08-28 17:02 ` Daniel Wagner
2023-08-28 18:55 ` Bart Van Assche
2023-08-29 2:11 ` Shinichiro Kawasaki
2023-08-29 13:35 ` Bart Van Assche
2023-08-30 1:19 ` Shinichiro Kawasaki
2023-09-01 0:33 ` Shinichiro Kawasaki
2023-08-25 6:24 ` Daniel Wagner
2023-08-28 4:17 ` Shinichiro Kawasaki
2023-08-28 15:14 ` Bart Van Assche
2023-08-28 17:03 ` Daniel Wagner
2023-08-22 17:54 ` [PATCH blktests v3 0/3] Introduce nvmet target setup/cleanup helpers Chaitanya Kulkarni
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®