| Message ID | 20240301094817.29491-3-dwagner@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | make nvme/048 checks more robust | expand |
On 3/1/24 01: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@suse.de> > --- > Looks good. Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com> -ck
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@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 }
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; + fi + retries=$((retries - 1)) + sleep 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 +85,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 }
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@suse.de> --- tests/nvme/048 | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)