On Aug 20, 2024 / 22:25, Nilay Shroff wrote:
>
>
> On 8/20/24 15:50, Shin'ichiro Kawasaki wrote:
[...]
> > diff --git a/tests/nvme/052 b/tests/nvme/052
> > index cf6061a..e1ac823 100755
> > --- a/tests/nvme/052
> > +++ b/tests/nvme/052
> > @@ -39,15 +39,32 @@ nvmf_wait_for_ns() {
> > ns=$(_find_nvme_ns "${uuid}")
> > done
> >
> > + echo "$ns"
> > return 0
> > }
> >
> > +nvmf_wait_for_ns_removal() {
> > + local ns=$1 i
> > +
> > + for ((i = 0; i < 10; i++)); do
> > + if [[ ! -e /dev/$ns ]]; then
> > + return
> > + fi
> > + sleep .1
> > + echo "wait removal of $ns" >> "$FULL"
> > + done
> > +
> > + if [[ -e /dev/$ns ]]; then
> > + echo "Failed to remove the namespace $ns"
> > + fi
> > +}
> > +
> Under nvmf_wait_for_ns_removal(), instead of checking the existence of "/dev/$ns",
> how about checking the existence of file "/sys/block/$ns"? As we know, when this issue
> manifests, we have a stale entry "/sys/block/$ns/$uuid" lurking around from the
> previous iteration for sometime causing the observed symptom. So I think, we may reuse the
> _find_nvme_ns() function to wait until the stale "/sys/block/$ns/$uuid" file
> exists.
It sounds a good idea to reuse _find_nvme_ns().
> Maybe something like below:
>
> nvmf_wait_for_ns_removal() {
> local ns
> local timeout="5"
> local uuid="$1"
>
> ns=$(_find_nvme_ns "${uuid}")
I tried this, and found that the _find_nvme_ns call spits out the failure
"cat: /sys/block/nvme1n2/uuid: No such file or directory", because the
delayed namespace removal can happen here. To suppress the error message,
this line should be,
ns=$(_find_nvme_ns "${uuid}" 2> /dev/null)
>
> start_time=$(date +%s)
> while [[ ! -z "$ns" ]]; do
> sleep 1
> end_time=$(date +%s)
> if (( end_time - start_time > timeout )); then
> echo "namespace with uuid \"${uuid}\" still " \
> "not deleted within ${timeout} seconds"
> return 1
> fi
> echo "Waiting for $ns removal" >> ${FULL}
> ns=$(_find_nvme_ns "${uuid}")
Same comment as above.
>
> done
>
> return 0
> }
I found that your nvmf_wait_for_ns_removal() above has certain amount of
duplication with the existing nvmf_wait_for_ns(). To avoid the duplication,
I suggest to reuse nvmf_wait_for_ns() and add a new argument to control wait
target event: namespace 'created' or 'removed'. With this idea, I created the
patch below. I confirmed the patch avoids the failure.
One drawback of this new patch based on your suggestion is that it extends
execution time of the test case from 20+ seconds to 40+ seconds. In most cases,
the while loop condition check in nvmf_wait_for_ns() is true at the first time,
and false at the the second time. So, nvmf_wait_for_ns() takes 1 second for one
time "sleep 1". Before applying this patch, it took 20+ seconds for 20 times
iteration. After applying the patch, it takes 40+ seconds, since one iteration
calls nvmf_wait_for_ns() twice. So how about to reduce the sleep time from 1 to
0.1? I tried it and observed that it reduced the runtime from 40+ seconds to 10+
seconds.
diff --git a/tests/nvme/052 b/tests/nvme/052
index cf6061a..22e0bf5 100755
--- a/tests/nvme/052
+++ b/tests/nvme/052
@@ -20,23 +20,35 @@ set_conditions() {
_set_nvme_trtype "$@"
}
+find_nvme_ns() {
+ if [[ "$2" == removed ]]; then
+ _find_nvme_ns "$1" 2> /dev/null
+ else
+ _find_nvme_ns "$1"
+ fi
+}
+
+# Wait for the namespace with specified uuid to fulfill the specified condtion,
+# "created" or "removed".
nvmf_wait_for_ns() {
local ns
local timeout="5"
local uuid="$1"
+ local condition="$2"
- ns=$(_find_nvme_ns "${uuid}")
+ ns=$(find_nvme_ns "${uuid}" "${condition}")
start_time=$(date +%s)
- while [[ -z "$ns" ]]; do
+ while [[ -z "$ns" && "$condition" == created ]] ||
+ [[ -n "$ns" && "$condition" == removed ]]; do
sleep 1
end_time=$(date +%s)
if (( end_time - start_time > timeout )); then
echo "namespace with uuid \"${uuid}\" not " \
- "found within ${timeout} seconds"
+ "${condition} within ${timeout} seconds"
return 1
fi
- ns=$(_find_nvme_ns "${uuid}")
+ ns=$(find_nvme_ns "${uuid}" "${condition}")
done
return 0
@@ -63,7 +75,7 @@ test() {
_create_nvmet_ns "${def_subsysnqn}" "${i}" "$(_nvme_def_file_path).$i" "${uuid}"
# wait until async request is processed and ns is created
- nvmf_wait_for_ns "${uuid}"
+ nvmf_wait_for_ns "${uuid}" created
if [ $? -eq 1 ]; then
echo "FAIL"
rm "$(_nvme_def_file_path).$i"
@@ -71,6 +83,10 @@ test() {
fi
_remove_nvmet_ns "${def_subsysnqn}" "${i}"
+
+ # wait until async request is processed and ns is removed
+ nvmf_wait_for_ns "${uuid}" removed
+
rm "$(_nvme_def_file_path).$i"
}
done
