Re: [PATCH 2/2] generic/500: skip this test if formatting fails

[Zorro Lang <zlang@redhat.com>]

On Tue, Jan 31, 2023 at 04:51:40PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> This testcase exercises what happens when we race a filesystem
> perforing discard operations against a thin provisioning device that has
> run out of space.  To constrain runtime, it creates a 128M thinp volume
> and formats it.
> 
> However, if that initial format fails because (say) the 128M volume is
> too small, then the test fails.  This is really a case of test
> preconditions not being satisfied, so let's make the test _notrun when
> this happens.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  common/dmthin     |    7 ++++-
>  common/rc         |   80 +++++++++++++++++++++++++++--------------------------
>  tests/generic/500 |    3 +-
>  3 files changed, 48 insertions(+), 42 deletions(-)
> 
> 
> diff --git a/common/dmthin b/common/dmthin
> index 91147e47ac..7107d50804 100644
> --- a/common/dmthin
> +++ b/common/dmthin
> @@ -234,5 +234,10 @@ _dmthin_mount()
>  _dmthin_mkfs()
>  {
>  	_scratch_options mkfs
> -	_mkfs_dev $SCRATCH_OPTIONS $@ $DMTHIN_VOL_DEV
> +	_mkfs_dev $SCRATCH_OPTIONS "$@" $DMTHIN_VOL_DEV

This patch adds quote marks to $@, that an extra change. So I hope to know if
this change fix something wrong or you just felt it's better to have?

If this part has a good explanation, others good to me, then I'd like to give
it:

Reviewed-by: Zorro Lang <zlang@redhat.com>

Thanks,
Zorro

> +}
> +_dmthin_try_mkfs()
> +{
> +	_scratch_options mkfs
> +	_try_mkfs_dev $SCRATCH_OPTIONS "$@" $DMTHIN_VOL_DEV
>  }
> diff --git a/common/rc b/common/rc
> index 36eb90e1f1..376a0138b4 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -604,49 +604,49 @@ _test_mkfs()
>      esac
>  }
>  
> +_try_mkfs_dev()
> +{
> +    case $FSTYP in
> +    nfs*)
> +	# do nothing for nfs
> +	;;
> +    9p)
> +	# do nothing for 9p
> +	;;
> +    fuse)
> +	# do nothing for fuse
> +	;;
> +    virtiofs)
> +	# do nothing for virtiofs
> +	;;
> +    overlay)
> +	# do nothing for overlay
> +	;;
> +    pvfs2)
> +	# do nothing for pvfs2
> +	;;
> +    udf)
> +        $MKFS_UDF_PROG $MKFS_OPTIONS $*
> +	;;
> +    btrfs)
> +        $MKFS_BTRFS_PROG $MKFS_OPTIONS $*
> +	;;
> +    ext2|ext3|ext4)
> +	$MKFS_PROG -t $FSTYP -- -F $MKFS_OPTIONS $*
> +	;;
> +    xfs)
> +	$MKFS_PROG -t $FSTYP -- -f $MKFS_OPTIONS $*
> +	;;
> +    *)
> +	yes | $MKFS_PROG -t $FSTYP -- $MKFS_OPTIONS $*
> +	;;
> +    esac
> +}
> +
>  _mkfs_dev()
>  {
>      local tmp=`mktemp -u`
> -    case $FSTYP in
> -    nfs*)
> -	# do nothing for nfs
> -	;;
> -    9p)
> -	# do nothing for 9p
> -	;;
> -    fuse)
> -	# do nothing for fuse
> -	;;
> -    virtiofs)
> -	# do nothing for virtiofs
> -	;;
> -    overlay)
> -	# do nothing for overlay
> -	;;
> -    pvfs2)
> -	# do nothing for pvfs2
> -	;;
> -    udf)
> -        $MKFS_UDF_PROG $MKFS_OPTIONS $* 2>$tmp.mkfserr 1>$tmp.mkfsstd
> -	;;
> -    btrfs)
> -        $MKFS_BTRFS_PROG $MKFS_OPTIONS $* 2>$tmp.mkfserr 1>$tmp.mkfsstd
> -	;;
> -    ext2|ext3|ext4)
> -	$MKFS_PROG -t $FSTYP -- -F $MKFS_OPTIONS $* \
> -		2>$tmp.mkfserr 1>$tmp.mkfsstd
> -	;;
> -    xfs)
> -	$MKFS_PROG -t $FSTYP -- -f $MKFS_OPTIONS $* \
> -		2>$tmp.mkfserr 1>$tmp.mkfsstd
> -	;;
> -    *)
> -	yes | $MKFS_PROG -t $FSTYP -- $MKFS_OPTIONS $* \
> -		2>$tmp.mkfserr 1>$tmp.mkfsstd
> -	;;
> -    esac
> -
> -    if [ $? -ne 0 ]; then
> +    if ! _try_mkfs_dev "$@" 2>$tmp.mkfserr 1>$tmp.mkfsstd; then
>  	# output stored mkfs output
>  	cat $tmp.mkfserr >&2
>  	cat $tmp.mkfsstd
> diff --git a/tests/generic/500 b/tests/generic/500
> index bc84d219fa..1151c8f234 100755
> --- a/tests/generic/500
> +++ b/tests/generic/500
> @@ -58,7 +58,8 @@ CLUSTER_SIZE=$((64 * 1024 / 512))		# 64K
>  
>  _dmthin_init $BACKING_SIZE $VIRTUAL_SIZE $CLUSTER_SIZE 0
>  _dmthin_set_fail
> -_dmthin_mkfs
> +_dmthin_try_mkfs >> $seqres.full 2>&1 || \
> +	_notrun "Could not format small thinp filesystem for test"
>  _dmthin_mount
>  
>  # There're two bugs at here, one is dm-thin bug, the other is filesystem
>