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

[Shinichiro Kawasaki]

On Mar 01, 2024 / 10:48, Daniel Wagner wrote:
> 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.
> 
> Signed-off-by: Daniel Wagner <dwagner at suse.de>

Hi Daniel, thanks for the patches. I did a trial run with fc transport and
confirmed that this patch avoids the nvme/048 failure. Good.

Please find nit comments below.

> ---
>  tests/nvme/048 | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/nvme/048 b/tests/nvme/048
> index 8c314fae9620..3b9a30bcca89 100755
> --- a/tests/nvme/048
> +++ b/tests/nvme/048
> @@ -47,11 +47,23 @@ 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
> +			    break;

The line above has extra spaces.

And I think the break above can be replaced with the echo and the return in the
if block after the while loop as follows. It will make the code a bit simpler.

diff --git a/tests/nvme/048 b/tests/nvme/048
index 3b9a30b..d6f3e75 100755
--- a/tests/nvme/048
+++ b/tests/nvme/048
@@ -56,7 +56,8 @@ nvmf_check_queue_count() {
 	queue_count_file=$(cat /sys/class/nvme-fabrics/ctl/"${nvmedev}"/queue_count)
 	while [[ "${queue_count}" -ne "${queue_count_file}" ]]; do
 		if [[ "${retries}" == 0 ]]; then
-			    break;
+			echo "expected queue count ${queue_count} not set"
+			return 1
 		fi
 		retries=$((retries - 1))
 		sleep 1
@@ -64,11 +65,6 @@ nvmf_check_queue_count() {
 		queue_count_file=$(cat /sys/class/nvme-fabrics/ctl/"${nvmedev}"/queue_count)
 	done
 
-	if [[ "${queue_count}" -ne "${queue_count_file}" ]]; then
-		echo "expected queue count ${queue_count} not set"
-		return 1
-	fi
-
 	return 0
 }