diff mbox series

[blktests,v2,4/4] nvme/061: add test teardown and setup fabrics target during I/O

Message ID 20250408-test-target-v2-4-e9e2512586f8@kernel.org (mailing list archive)
State New
Headers show
Series blktests: add target test cases | expand

Commit Message

Daniel Wagner April 8, 2025, 4:26 p.m. UTC 
Add a new test case which forcefully removes the target and setup it
again.

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
Signed-off-by: Daniel Wagner <wagi@kernel.org>
---
 tests/nvme/061     | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/nvme/061.out | 21 +++++++++++++++++
 2 files changed, 87 insertions(+)

Comments

Shinichiro Kawasaki April 14, 2025, 12:18 p.m. UTC | #1 
On Apr 08, 2025 / 18:26, Daniel Wagner wrote:
> Add a new test case which forcefully removes the target and setup it
> again.
> 
> Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
> Signed-off-by: Daniel Wagner <wagi@kernel.org>

When I ran this test case for tr=loop, it fails.

The out file is as follows:

  $ cat ./results/nodev_tr_loop/nvme/061.out.bad
  Running nvme/061
  iteration 0
  grep: /sys/class/nvme-fabrics/ctl//state: No such file or directory
  grep: /sys/class/nvme-fabrics/ctl//state: No such file or directory
  grep: /sys/class/nvme-fabrics/ctl//state: No such file or directory
  grep: /sys/class/nvme-fabrics/ctl//state: No such file or directory
  grep: /sys/class/nvme-fabrics/ctl//state: No such file or directory
  grep: /sys/class/nvme-fabrics/ctl//state: No such file or directory
  expected state "connecting" not  reached within 5 seconds

And kernel reported the "invalid parameter" message:

  [  888.896492][ T3112] run blktests nvme/061 at 2025-04-14 21:11:18
  [  888.937128][ T3158] loop0: detected capacity change from 0 to 2097152
  [  888.949671][ T3161] nvmet: adding nsid 1 to subsystem blktests-subsystem-1
  [  888.997790][ T3170] nvme_fabrics: invalid parameter 'reconnect_delay=%d'

In the v1 series, you noted that your fc-loop fixes will avoid the failure.
But the failure was observed with tr=loop, so I'm not sure fc-loop fixes
will avoids the failure. I'm wondering if this test case is for rdma/tcp/fc
transports only and suspecting it may not be intended for the loop transport.

Also, please find nit comments in line:

> ---
>  tests/nvme/061     | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/nvme/061.out | 21 +++++++++++++++++
>  2 files changed, 87 insertions(+)
> 
> diff --git a/tests/nvme/061 b/tests/nvme/061
> new file mode 100755
> index 0000000000000000000000000000000000000000..3b59d7137932258d06c3d64166db493df176ba4e
> --- /dev/null
> +++ b/tests/nvme/061
> @@ -0,0 +1,66 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2025 Daniel Wagner, SUSE Labs
> +#
> +# Test if the host keeps running IO when the target is forcefully removed and
> +# added back.
> +
> +. tests/nvme/rc
> +
> +DESCRIPTION="test fabric target teardown and setup during I/O"
> +TIMED=1
> +
> +requires() {
> +	_nvme_requires
> +	_have_loop
> +	_have_fio
> +	_require_nvme_trtype_is_fabrics
> +}
> +
> +set_conditions() {
> +	_set_nvme_trtype "$@"
> +}
> +
> +test() {
> +	echo "Running ${TEST_NAME}"
> +
> +	_setup_nvmet
> +
> +	local ns
> +
> +	_nvmet_target_setup
> +
> +	_nvme_connect_subsys --keep-alive-tmo 1 --reconnect-delay 1
> +
> +	ns=$(_find_nvme_ns "${def_subsys_uuid}")
> +
> +	_run_fio_rand_io --filename="/dev/${ns}" \
> +		--group_reporting \
> +		--time_based --runtime=1d &> /dev/null &
> +	fio_pid=$!
> +	sleep 1
> +
> +	nvmedev=$(_find_nvme_dev "${def_subsysnqn}")
> +	state_file="/sys/class/nvme-fabrics/ctl/${nvmedev}/state"
> +	for ((i = 0; i <= 5; i++)); do
> +		echo "iteration $i"
> +
> +		_nvmet_target_cleanup
> +
> +		_nvmf_wait_for_state "${def_subsysnqn}" "connecting" || return 1
> +		echo "state: $(cat ${state_file})"

The line above needs one more pair of double quotations to avoid the
shellcheck warn:

		echo "state: $(cat "${state_file}")"

> +
> +		_nvmet_target_setup
> +
> +		_nvmf_wait_for_state "${def_subsysnqn}" "live" || return 1
> +		echo "state: $(cat ${state_file})"

Same here:

		echo "state: $(cat "${state_file}")"
Daniel Wagner April 14, 2025, 1:27 p.m. UTC | #2 
On Mon, Apr 14, 2025 at 12:18:05PM +0000, Shinichiro Kawasaki wrote:
> On Apr 08, 2025 / 18:26, Daniel Wagner wrote:
> > Add a new test case which forcefully removes the target and setup it
> > again.
> > 
> > Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
> > Signed-off-by: Daniel Wagner <wagi@kernel.org>
> 
> When I ran this test case for tr=loop, it fails.
> 
> The out file is as follows:
> 
>   $ cat ./results/nodev_tr_loop/nvme/061.out.bad
>   Running nvme/061
>   iteration 0
>   grep: /sys/class/nvme-fabrics/ctl//state: No such file or directory
>   grep: /sys/class/nvme-fabrics/ctl//state: No such file or directory
>   grep: /sys/class/nvme-fabrics/ctl//state: No such file or directory
>   grep: /sys/class/nvme-fabrics/ctl//state: No such file or directory
>   grep: /sys/class/nvme-fabrics/ctl//state: No such file or directory
>   grep: /sys/class/nvme-fabrics/ctl//state: No such file or directory
>   expected state "connecting" not  reached within 5 seconds
> 
> And kernel reported the "invalid parameter" message:
> 
>   [  888.896492][ T3112] run blktests nvme/061 at 2025-04-14 21:11:18
>   [  888.937128][ T3158] loop0: detected capacity change from 0 to 2097152
>   [  888.949671][ T3161] nvmet: adding nsid 1 to subsystem blktests-subsystem-1
>   [  888.997790][ T3170] nvme_fabrics: invalid parameter 'reconnect_delay=%d'
> 
> In the v1 series, you noted that your fc-loop fixes will avoid the
> failure.

Yes, the fcloop fixes will address the fc transport failures. The above
failure is caused by the same issue as in 060: the loop transport
doesn't know about reconnect_delay, thus the requires should list tcp,
rdma and fc as valid transport and not loop.

> But the failure was observed with tr=loop, so I'm not sure fc-loop fixes
> will avoids the failure. I'm wondering if this test case is for rdma/tcp/fc
> transports only and suspecting it may not be intended for the loop
> transport.

Yes, this is correct. My test script listed only tcp, rdma and fc, so I
didn't catch the error with loop.

> > +	nvmedev=$(_find_nvme_dev "${def_subsysnqn}")
> > +	state_file="/sys/class/nvme-fabrics/ctl/${nvmedev}/state"
> > +	for ((i = 0; i <= 5; i++)); do
> > +		echo "iteration $i"
> > +
> > +		_nvmet_target_cleanup
> > +
> > +		_nvmf_wait_for_state "${def_subsysnqn}" "connecting" || return 1
> > +		echo "state: $(cat ${state_file})"
> 
> The line above needs one more pair of double quotations to avoid the
> shellcheck warn:
> 
> 		echo "state: $(cat "${state_file}")"

I didn't know this is actually allowed and even required. Sure, will
update the patches accordingly.

Thanks,
Daniel
diff mbox series

Patch

diff --git a/tests/nvme/061 b/tests/nvme/061
new file mode 100755
index 0000000000000000000000000000000000000000..3b59d7137932258d06c3d64166db493df176ba4e
--- /dev/null
+++ b/tests/nvme/061
@@ -0,0 +1,66 @@ 
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2025 Daniel Wagner, SUSE Labs
+#
+# Test if the host keeps running IO when the target is forcefully removed and
+# added back.
+
+. tests/nvme/rc
+
+DESCRIPTION="test fabric target teardown and setup during I/O"
+TIMED=1
+
+requires() {
+	_nvme_requires
+	_have_loop
+	_have_fio
+	_require_nvme_trtype_is_fabrics
+}
+
+set_conditions() {
+	_set_nvme_trtype "$@"
+}
+
+test() {
+	echo "Running ${TEST_NAME}"
+
+	_setup_nvmet
+
+	local ns
+
+	_nvmet_target_setup
+
+	_nvme_connect_subsys --keep-alive-tmo 1 --reconnect-delay 1
+
+	ns=$(_find_nvme_ns "${def_subsys_uuid}")
+
+	_run_fio_rand_io --filename="/dev/${ns}" \
+		--group_reporting \
+		--time_based --runtime=1d &> /dev/null &
+	fio_pid=$!
+	sleep 1
+
+	nvmedev=$(_find_nvme_dev "${def_subsysnqn}")
+	state_file="/sys/class/nvme-fabrics/ctl/${nvmedev}/state"
+	for ((i = 0; i <= 5; i++)); do
+		echo "iteration $i"
+
+		_nvmet_target_cleanup
+
+		_nvmf_wait_for_state "${def_subsysnqn}" "connecting" || return 1
+		echo "state: $(cat ${state_file})"
+
+		_nvmet_target_setup
+
+		_nvmf_wait_for_state "${def_subsysnqn}" "live" || return 1
+		echo "state: $(cat ${state_file})"
+	done
+
+	{ kill "${fio_pid}"; wait; } &> /dev/null
+
+	_nvme_disconnect_subsys
+
+	_nvmet_target_cleanup
+
+	echo "Test complete"
+}
diff --git a/tests/nvme/061.out b/tests/nvme/061.out
new file mode 100644
index 0000000000000000000000000000000000000000..75516abdac005854c2be165005c076ef8891c518
--- /dev/null
+++ b/tests/nvme/061.out
@@ -0,0 +1,21 @@ 
+Running nvme/061
+iteration 0
+state: connecting
+state: live
+iteration 1
+state: connecting
+state: live
+iteration 2
+state: connecting
+state: live
+iteration 3
+state: connecting
+state: live
+iteration 4
+state: connecting
+state: live
+iteration 5
+state: connecting
+state: live
+disconnected 1 controller(s)
+Test complete