| Message ID | 20240925073245.241234-1-nilay@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [PATCHv2,blktests] nvme/{033-037}: timeout while waiting for nvme passthru namespace device | expand |
On Sep 25, 2024 / 13:01, Nilay Shroff wrote: > Avoid waiting indefinitely for nvme passthru namespace block device > to appear. Wait for up to 5 seconds and during this time if namespace > device doesn't appear then bail out and FAIL the test. > > Signed-off-by: Nilay Shroff <nilay@linux.ibm.com> > --- > Changes from v1: > - Add a meaningful error message if test fails (Shnichiro > Kawasaki) > - Use sleep "1" instead of ".1" while waiting for nsdev to be > created as we don't see much gain in test runtime with short > duration of sleep. This would also help further optimize > the sleep logic (Shnichiro Kawasaki) > - Few other trivial cleanups (Shnichiro Kawasaki) Thanks for this v2 patch. [...] > diff --git a/tests/nvme/036 b/tests/nvme/036 > index 442ffe7..a114a7c 100755 > --- a/tests/nvme/036 > +++ b/tests/nvme/036 > @@ -27,6 +27,7 @@ test_device() { > _setup_nvmet > > local ctrldev > + local nsdev > > _nvmet_passthru_target_setup > nsdev=$(_nvmet_passthru_target_connect) I commented for v1 that "unnecsseary change" was made for nmve/036. With that comment, I meant that one empty line removal was unnecessary. However, you removed the nsdev check in nvme/036 for v2. I guess you might have misunderstood my comment then removed the check. I suggest to put back the nsdev check in nvme/036. If you agree, could you send out v3? Or I can do it when I apply this patch. Other than that, this patch looks good to me. Let's wait and see if anyone has other comments.
On 9/27/24 07:41, Shinichiro Kawasaki wrote: > On Sep 25, 2024 / 13:01, Nilay Shroff wrote: >> Avoid waiting indefinitely for nvme passthru namespace block device >> to appear. Wait for up to 5 seconds and during this time if namespace >> device doesn't appear then bail out and FAIL the test. >> >> Signed-off-by: Nilay Shroff <nilay@linux.ibm.com> >> --- >> Changes from v1: >> - Add a meaningful error message if test fails (Shnichiro >> Kawasaki) >> - Use sleep "1" instead of ".1" while waiting for nsdev to be >> created as we don't see much gain in test runtime with short >> duration of sleep. This would also help further optimize >> the sleep logic (Shnichiro Kawasaki) >> - Few other trivial cleanups (Shnichiro Kawasaki) > > Thanks for this v2 patch. > > [...] > >> diff --git a/tests/nvme/036 b/tests/nvme/036 >> index 442ffe7..a114a7c 100755 >> --- a/tests/nvme/036 >> +++ b/tests/nvme/036 >> @@ -27,6 +27,7 @@ test_device() { >> _setup_nvmet >> >> local ctrldev >> + local nsdev >> >> _nvmet_passthru_target_setup >> nsdev=$(_nvmet_passthru_target_connect) > > I commented for v1 that "unnecsseary change" was made for nmve/036. With that > comment, I meant that one empty line removal was unnecessary. However, you > removed the nsdev check in nvme/036 for v2. I guess you might have misunderstood > my comment then removed the check. I suggest to put back the nsdev check in > nvme/036. If you agree, could you send out v3? Or I can do it when I apply this > patch. > Ah, yes I think that was misunderstanding. When I reviewed your last comment about nvme/036, I thought you wanted to remove nsdev check just because the intention of the nvme/036 test was to test "reset controller" command. So in that sense, the nsdev check is not important. Having said that, yes keeping nsdev check may also not harm. So I would add that nsdev check back in nvme/036 and send out patch v3. > Other than that, this patch looks good to me. Let's wait and see if anyone has > other comments. Thanks, --Nilay
diff --git a/tests/nvme/033 b/tests/nvme/033 index 5e05175..d0581c2 100755 --- a/tests/nvme/033 +++ b/tests/nvme/033 @@ -62,8 +62,11 @@ test_device() { _nvmet_passthru_target_setup nsdev=$(_nvmet_passthru_target_connect) - - compare_dev_info "${nsdev}" + if [[ -z "$nsdev" ]]; then + echo "FAIL: Failed to find passthru target namespace" + else + compare_dev_info "${nsdev}" + fi _nvme_disconnect_subsys _nvmet_passthru_target_cleanup diff --git a/tests/nvme/034 b/tests/nvme/034 index 154fc91..a4c5e97 100755 --- a/tests/nvme/034 +++ b/tests/nvme/034 @@ -27,13 +27,15 @@ test_device() { _setup_nvmet - local ctrldev local nsdev _nvmet_passthru_target_setup nsdev=$(_nvmet_passthru_target_connect) - - _run_fio_verify_io --size="${NVME_IMG_SIZE}" --filename="${nsdev}" + if [[ -z "$nsdev" ]]; then + echo "FAIL: Failed to find passthru target namespace" + else + _run_fio_verify_io --size="${NVME_IMG_SIZE}" --filename="${nsdev}" + fi _nvme_disconnect_subsys _nvmet_passthru_target_cleanup diff --git a/tests/nvme/035 b/tests/nvme/035 index ff217d6..9f84ced 100755 --- a/tests/nvme/035 +++ b/tests/nvme/035 @@ -30,13 +30,13 @@ test_device() { _setup_nvmet - local ctrldev local nsdev _nvmet_passthru_target_setup nsdev=$(_nvmet_passthru_target_connect) - - if ! _xfs_run_fio_verify_io "${nsdev}" "${NVME_IMG_SIZE}"; then + if [[ -z "$nsdev" ]]; then + echo "FAIL: Failed to find passthru target namespace" + elif ! _xfs_run_fio_verify_io "${nsdev}" "${NVME_IMG_SIZE}"; then echo "FAIL: fio verify failed" fi diff --git a/tests/nvme/036 b/tests/nvme/036 index 442ffe7..a114a7c 100755 --- a/tests/nvme/036 +++ b/tests/nvme/036 @@ -27,6 +27,7 @@ test_device() { _setup_nvmet local ctrldev + local nsdev _nvmet_passthru_target_setup nsdev=$(_nvmet_passthru_target_connect) diff --git a/tests/nvme/037 b/tests/nvme/037 index f7ddc2d..33a6857 100755 --- a/tests/nvme/037 +++ b/tests/nvme/037 @@ -27,7 +27,7 @@ test_device() { local subsys="blktests-subsystem-" local iterations=10 - local ctrldev + local nsdev for ((i = 0; i < iterations; i++)); do _nvmet_passthru_target_setup --subsysnqn "${subsys}${i}" @@ -37,6 +37,11 @@ test_device() { _nvme_disconnect_subsys \ --subsysnqn "${subsys}${i}" >>"${FULL}" 2>&1 _nvmet_passthru_target_cleanup --subsysnqn "${subsys}${i}" + + if [[ -z "$nsdev" ]]; then + echo "FAIL: Failed to find passthru target namespace" + break + fi done echo "Test complete" diff --git a/tests/nvme/rc b/tests/nvme/rc index a877de3..671012e 100644 --- a/tests/nvme/rc +++ b/tests/nvme/rc @@ -394,6 +394,8 @@ _nvmet_passthru_target_setup() { _nvmet_passthru_target_connect() { local subsysnqn="$def_subsysnqn" + local timeout="5" + local count="0" while [[ $# -gt 0 ]]; do case $1 in @@ -414,7 +416,12 @@ _nvmet_passthru_target_connect() { # The following tests can race with the creation # of the device so ensure the block device exists # before continuing - while [ ! -b "${nsdev}" ]; do sleep 1; done + while [ ! -b "${nsdev}" ]; do + sleep 1 + if ((++count >= timeout)); then + return 1 + fi + done echo "${nsdev}" }
Avoid waiting indefinitely for nvme passthru namespace block device to appear. Wait for up to 5 seconds and during this time if namespace device doesn't appear then bail out and FAIL the test. Signed-off-by: Nilay Shroff <nilay@linux.ibm.com> --- Changes from v1: - Add a meaningful error message if test fails (Shnichiro Kawasaki) - Use sleep "1" instead of ".1" while waiting for nsdev to be created as we don't see much gain in test runtime with short duration of sleep. This would also help further optimize the sleep logic (Shnichiro Kawasaki) - Few other trivial cleanups (Shnichiro Kawasaki) --- tests/nvme/033 | 7 +++++-- tests/nvme/034 | 8 +++++--- tests/nvme/035 | 6 +++--- tests/nvme/036 | 1 + tests/nvme/037 | 7 ++++++- tests/nvme/rc | 9 ++++++++- 6 files changed, 28 insertions(+), 10 deletions(-)