Re: [PATCH] test: Retry umount if necessary

[Kris Van Hees <kris.van.hees@oracle.com>]

On Tue, Dec 03, 2024 at 03:16:01PM -0500, eugene.loh@oracle.com wrote:
> From: Eugene Loh <eugene.loh@oracle.com>
> 
> The io tests tst.local.sh and tst.wait.sh fail with some frequency,
> with error messages like "umount: $iodir: target is busy."
> 
> Modify the tests' dtrace trigger, doio.sh, so that it will retry
> umount a few times, if necessary.  Specifically, remove the "set -e"
> and individually check for and report errors, in the case of umount
> retrying a few times before giving up.
> 
> While we're at it, notice that the two tests use the same $iodir.
> There is no need for the tests to be coupled in this way.  Change
> those two tests so that each test has a fresh value of iodir.
> 
> Signed-off-by: Eugene Loh <eugene.loh@oracle.com>

Reviewed-by: Kris Van Hees <kris.van.hees@oracle.com>

... I did read the discussion about lazy umount, but I think that in terms
of what is best for the testsuite, being able to ascertain that the umount
is actually getting done (or reporting an error on it) is important in case
later tests might get affected by it.  Lazy umount seems like it would lead
to possible silent umount conditions which is not ideal for a test.

> ---
>  test/triggers/doio.sh         | 29 ++++++++++++++++++++++++++---
>  test/unittest/io/tst.local.sh |  2 +-
>  test/unittest/io/tst.wait.sh  |  2 +-
>  3 files changed, 28 insertions(+), 5 deletions(-)
> 
> diff --git a/test/triggers/doio.sh b/test/triggers/doio.sh
> index a2a39c245..0171a4b4c 100755
> --- a/test/triggers/doio.sh
> +++ b/test/triggers/doio.sh
> @@ -22,15 +22,38 @@ mountdir=$4
>  mountarg1=${5-""}
>  mountarg2=${6-""}
>  
> -set -e
> -
>  # do writes
>  dd if=/dev/urandom of=$tempfile count=$filesize bs=1 status=none
> +if [ $? -ne 0 ]; then
> +	echo ERROR dd
> +	exit 1
> +fi
>  
>  # flush cache and remount the file system to force the IO
> -umount $mountdir
> +ntries=3
> +while [ $ntries -gt 0 ]; do
> +	umount $mountdir >& /dev/null
> +	if [ $? -eq 0 ]; then
> +		break
> +	fi
> +	sleep 1
> +	ntries=$(($ntries - 1))
> +done
> +if [ $ntries -eq 0 ]; then
> +	echo ERROR umount
> +	exit 1
> +fi
> +
>  echo 3 > /proc/sys/vm/drop_caches
>  $mountcmd $mountdir $mountarg1 $mountarg2
> +if [ $? -ne 0 ]; then
> +	echo ERROR $mountcmd
> +	exit 1
> +fi
>  
>  # do reads
>  sum $tempfile > /dev/null
> +if [ $? -ne 0 ]; then
> +	echo ERROR sum
> +	exit 1
> +fi
> diff --git a/test/unittest/io/tst.local.sh b/test/unittest/io/tst.local.sh
> index d3dbf1713..702c6f453 100755
> --- a/test/unittest/io/tst.local.sh
> +++ b/test/unittest/io/tst.local.sh
> @@ -18,7 +18,7 @@ minsize=$((filesize / 10 * 9))
>  fstype=xfs
>  # file system-specific options
>  fsoptions="defaults,atime,diratime,nosuid,nodev"
> -iodir=$tmpdir/test-$fstype-io
> +iodir=$tmpdir/test-$fstype-io-local.$$
>  tempfile=`mktemp -u -p $iodir`
>  
>  trap "umount $iodir; rmdir $iodir; rm -f $iodir.img" QUIT EXIT
> diff --git a/test/unittest/io/tst.wait.sh b/test/unittest/io/tst.wait.sh
> index 016b922eb..24ac2e436 100755
> --- a/test/unittest/io/tst.wait.sh
> +++ b/test/unittest/io/tst.wait.sh
> @@ -15,7 +15,7 @@ filesize=$((1024*$nblocks))
>  fstype=xfs
>  # file system-specific options
>  fsoptions="defaults,atime,diratime,nosuid,nodev"
> -iodir=$tmpdir/test-$fstype-io
> +iodir=$tmpdir/test-$fstype-io-wait.$$
>  tempfile=`mktemp -u -p $iodir`
>  
>  trap "umount $iodir; rmdir $iodir; rm -f $iodir.img" QUIT EXIT
> -- 
> 2.43.5
>