| Message ID | 20230405154630.16298-5-dwagner@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | test queue count changes on reconnect | expand |
On 4/5/23 08:46, Daniel Wagner wrote: > The target is allowed to change the number of I/O queues. Test if the > host is able to reconnect in this scenario. > > Signed-off-by: Daniel Wagner <dwagner@suse.de> > --- > Looks good. Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com> -ck
> + if ! _detect_nvmet_subsys_attr "attr_qid_max"; then > + SKIP_REASONS+=("missing attr_qid_max feature") > + return 1 > + fi > + > + truncate -s 512M "${file_path}" > + > + _create_nvmet_subsystem "${subsys_name}" "${file_path}" \ > + "b92842df-a394-44b1-84a4-92ae7d112861" by checking following after create subsystem in testcase itself we avoid whole process of creating and deleting subsystem and additional function in the rc file, because we are already creating subsystem as a part of the testcase :- local attr="${NVMET_CFS}/subsystems/${subsys_name}/attr_qid_max" #above tow vars go top of this function if [ -f "${attr}" ];then SKIP_REASONS+=("missing attr_qid_max feature") #do appropriate error handling and jump to unwind code fi again please ignore this comment if decision has been made to keep it this way for some reason... > + port="$(_create_nvmet_port "${nvme_trtype}")" > + _add_nvmet_subsys_to_port "${port}" "${subsys_name}" > + _create_nvmet_host "${subsys_name}" "${hostnqn}" > + -ck
On Wed, Apr 05, 2023 at 06:57:49PM +0000, Chaitanya Kulkarni wrote: > > > + if ! _detect_nvmet_subsys_attr "attr_qid_max"; then > > + SKIP_REASONS+=("missing attr_qid_max feature") > > + return 1 > > + fi > > + > > + truncate -s 512M "${file_path}" > > + > > + _create_nvmet_subsystem "${subsys_name}" "${file_path}" \ > > + "b92842df-a394-44b1-84a4-92ae7d112861" > > by checking following after create subsystem in testcase itself > we avoid whole process of creating and deleting subsystem and > additional function in the rc file, because we are already creating > subsystem as a part of the testcase :- > > local attr="${NVMET_CFS}/subsystems/${subsys_name}/attr_qid_max" > > #above tow vars go top of this function > > if [ -f "${attr}" ];then > SKIP_REASONS+=("missing attr_qid_max feature") > #do appropriate error handling and jump to unwind code > fi > > again please ignore this comment if decision has been made to > keep it this way for some reason... Again, no decision here. I think I overengineered this part slightly. Indeed if we are goint to setup a controller anyway we should try to avoid double work. This should also speed up the test slightly. Talking about execution time, I was thinking on reducing the timeout value to reduce the overall runtime.
On 4/5/23 23:17, Daniel Wagner wrote: > On Wed, Apr 05, 2023 at 06:57:49PM +0000, Chaitanya Kulkarni wrote: >>> + if ! _detect_nvmet_subsys_attr "attr_qid_max"; then >>> + SKIP_REASONS+=("missing attr_qid_max feature") >>> + return 1 >>> + fi >>> + >>> + truncate -s 512M "${file_path}" >>> + >>> + _create_nvmet_subsystem "${subsys_name}" "${file_path}" \ >>> + "b92842df-a394-44b1-84a4-92ae7d112861" >> by checking following after create subsystem in testcase itself >> we avoid whole process of creating and deleting subsystem and >> additional function in the rc file, because we are already creating >> subsystem as a part of the testcase :- >> >> local attr="${NVMET_CFS}/subsystems/${subsys_name}/attr_qid_max" >> >> #above tow vars go top of this function >> >> if [ -f "${attr}" ];then >> SKIP_REASONS+=("missing attr_qid_max feature") >> #do appropriate error handling and jump to unwind code >> fi >> >> again please ignore this comment if decision has been made to >> keep it this way for some reason... > Again, no decision here. I think I overengineered this part slightly. Indeed if > we are goint to setup a controller anyway we should try to avoid double work. > This should also speed up the test slightly. > > Talking about execution time, I was thinking on reducing the timeout value > to reduce the overall runtime. okay let's go with the above suggestion than ? and overall let's only add to rc when there are multiple users to the function with prep patches moving the code, otherwise it gets bloated for no reason.. overall speed of the testcase is also very important since this gets run with bunch of other testcases/testsuite(s) so keeping the testcase speed minimum is always desirable ... -ck
diff --git a/tests/nvme/048 b/tests/nvme/048 new file mode 100755 index 000000000000..bc4766e82159 --- /dev/null +++ b/tests/nvme/048 @@ -0,0 +1,81 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-3.0+ +# Copyright (C) 2023 Daniel Wagner, SUSE Labs +# +# Test queue count changes on reconnect + +. tests/nvme/rc + +DESCRIPTION="Test queue count changes on reconnect" +QUICK=1 + +requires() { + _nvme_requires + _have_loop + _require_nvme_trtype tcp rdma fc + _require_min_cpus 2 +} + +set_max_qid() { + local port="$1" + local subsys_name="$2" + local max_qid="$3" + + _set_nvmet_attr_qid_max "${subsys_name}" "${max_qid}" + + # Setting qid_max forces a disconnect and the reconntect attempt starts + # 10s after connection lost, thus we will see the connecting state. + _nvmf_wait_for_state "${subsys_name}" "connecting" + _nvmf_wait_for_state "${subsys_name}" "live" +} + +test() { + local port + local subsys_name="blktests-subsystem-1" + local hostid + local hostnqn="nqn.2014-08.org.nvmexpress:uuid:${hostid}" + local file_path="${TMPDIR}/img" + + echo "Running ${TEST_NAME}" + + hostid="$(uuidgen)" + if [ -z "$hostid" ] ; then + echo "uuidgen failed" + return 1 + fi + + if ! _detect_nvmet_subsys_attr "attr_qid_max"; then + SKIP_REASONS+=("missing attr_qid_max feature") + return 1 + fi + + truncate -s 512M "${file_path}" + + _create_nvmet_subsystem "${subsys_name}" "${file_path}" \ + "b92842df-a394-44b1-84a4-92ae7d112861" + port="$(_create_nvmet_port "${nvme_trtype}")" + _add_nvmet_subsys_to_port "${port}" "${subsys_name}" + _create_nvmet_host "${subsys_name}" "${hostnqn}" + + _nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" \ + --hostnqn "${hostnqn}" \ + --hostid "${hostid}" + + _nvmf_wait_for_state "${subsys_name}" "live" + + set_max_qid "${port}" "${subsys_name}" 1 + set_max_qid "${port}" "${subsys_name}" 128 + + _nvme_disconnect_subsys "${subsys_name}" + + _remove_nvmet_subsystem_from_port "${port}" "${subsys_name}" + _remove_nvmet_subsystem "${subsys_name}" + + _remove_nvmet_port "${port}" + + _remove_nvmet_host "${hostnqn}" + + rm "${file_path}" + + echo "Test complete" +} diff --git a/tests/nvme/048.out b/tests/nvme/048.out new file mode 100644 index 000000000000..7f986ef9637d --- /dev/null +++ b/tests/nvme/048.out @@ -0,0 +1,3 @@ +Running nvme/048 +NQN:blktests-subsystem-1 disconnected 1 controller(s) +Test complete
The target is allowed to change the number of I/O queues. Test if the host is able to reconnect in this scenario. Signed-off-by: Daniel Wagner <dwagner@suse.de> --- tests/nvme/048 | 81 ++++++++++++++++++++++++++++++++++++++++++++++ tests/nvme/048.out | 3 ++ 2 files changed, 84 insertions(+) create mode 100755 tests/nvme/048 create mode 100644 tests/nvme/048.out