[PATCH blktests v2 0/2] make nvme/048 checks more robust

[PATCH blktests v2 0/2] make nvme/048 checks more robust

[Daniel Wagner]

The nvme/048 tests fails with the fc transport because the check logic is racing
with the reset code in the target. By making the checks a bit more robust in
blktests, this test passes for all transports.

changes
v2:
  - added RB tags
  - refactored exit path as suggested by Shinichiro
v1:
  - initial version
  - https://lore.kernel.org/linux-nvme/20240301094817.29491-1-dwagner@suse.de/

*** BLURB HERE ***

Daniel Wagner (2):
  nvme/048: remove unused argument for set_qid_max
  nvme/048: make queue count check retry-able

 tests/nvme/048 | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

-- 
2.44.0

[PATCH blktests v2 1/2] nvme/048: remove unused argument for set_qid_max

[Daniel Wagner]

The port is argument is unsed, thus remove it.

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
Signed-off-by: Daniel Wagner <dwagner@suse.de>
---
 tests/nvme/048 | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/tests/nvme/048 b/tests/nvme/048
index 1e5a7a1bcb99..8c314fae9620 100755
--- a/tests/nvme/048
+++ b/tests/nvme/048
@@ -69,9 +69,8 @@ set_nvmet_attr_qid_max() {
 }
 
 set_qid_max() {
-	local port="$1"
-	local subsys_name="$2"
-	local qid_max="$3"
+	local subsys_name="$1"
+	local qid_max="$2"
 
 	set_nvmet_attr_qid_max "${subsys_name}" "${qid_max}"
 	nvmf_wait_for_state "${subsys_name}" "live" || return 1
@@ -100,8 +99,8 @@ test() {
 		if ! nvmf_wait_for_state "${def_subsysnqn}" "live" ; then
 			echo FAIL
 		else
-			set_qid_max "${port}" "${def_subsysnqn}" 1 || echo FAIL
-			set_qid_max "${port}" "${def_subsysnqn}" 2 || echo FAIL
+			set_qid_max "${def_subsysnqn}" 1 || echo FAIL
+			set_qid_max "${def_subsysnqn}" 2 || echo FAIL
 		fi
 
 		_nvme_disconnect_subsys "${def_subsysnqn}"
-- 
2.44.0

[PATCH blktests v2 2/2] nvme/048: make queue count check retry-able

[Daniel Wagner]

We are racing with the reset path of the controller. That means, when we
set a new queue count, we might not observe the resetting state in time.
Thus, first check if we see the correct queue count and then the
controller state.

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
Signed-off-by: Daniel Wagner <dwagner@suse.de>
---
 tests/nvme/048 | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/tests/nvme/048 b/tests/nvme/048
index 8c314fae9620..393dbc2a07d5 100755
--- a/tests/nvme/048
+++ b/tests/nvme/048
@@ -47,11 +47,25 @@ nvmf_check_queue_count() {
 	local queue_count="$2"
 	local nvmedev
 	local queue_count_file
+	local retries
 
 	nvmedev=$(_find_nvme_dev "${subsys_name}")
+	queue_count=$((queue_count + 1))
+	retries=5
+
 	queue_count_file=$(cat /sys/class/nvme-fabrics/ctl/"${nvmedev}"/queue_count)
+	while [[ "${queue_count}" -ne "${queue_count_file}" ]]; do
+		if [[ "${retries}" == 0 ]]; then
+			echo "expected queue count ${queue_count} not set"
+			return 1
+		fi
+
+		sleep 1
+
+		retries=$((retries - 1))
+		queue_count_file=$(cat /sys/class/nvme-fabrics/ctl/"${nvmedev}"/queue_count)
+	done
 
-	queue_count=$((queue_count + 1))
 	if [[ "${queue_count}" -ne "${queue_count_file}" ]]; then
 		echo "expected queue count ${queue_count} not set"
 		return 1
@@ -73,8 +87,8 @@ set_qid_max() {
 	local qid_max="$2"
 
 	set_nvmet_attr_qid_max "${subsys_name}" "${qid_max}"
-	nvmf_wait_for_state "${subsys_name}" "live" || return 1
 	nvmf_check_queue_count "${subsys_name}" "${qid_max}" || return 1
+	nvmf_wait_for_state "${subsys_name}" "live" || return 1
 
 	return 0
 }
-- 
2.44.0

Re: [PATCH blktests v2 0/2] make nvme/048 checks more robust

[Shinichiro Kawasaki]

On Mar 04, 2024 / 14:48, Daniel Wagner wrote:
> The nvme/048 tests fails with the fc transport because the check logic is racing
> with the reset code in the target. By making the checks a bit more robust in
> blktests, this test passes for all transports.
> 
> changes
> v2:
>   - added RB tags
>   - refactored exit path as suggested by Shinichiro
> v1:
>   - initial version
>   - https://lore.kernel.org/linux-nvme/20240301094817.29491-1-dwagner@suse.de/

Thanks for this v2 series. I've applied the two patches.

Of note is that I removed the left if block in nvmf_check_queue_count(), which I
believe unnecessary.

Re: Re: [PATCH blktests v2 0/2] make nvme/048 checks more robust

[Daniel Wagner]

On Tue, Mar 05, 2024 at 01:29:18AM +0000, Shinichiro Kawasaki wrote:
> Of note is that I removed the left if block in nvmf_check_queue_count(), which I
> believe unnecessary.

This was also what I did. But somehow I managed to send out the wrong
version. Thanks for fixing it!