Re: [PATCH v2 2/3] xfs/realtime: Default rtinherit=1, add _require_no_rtinherit function

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Mon, Sep 25, 2017 at 12:56:24PM -0700, Richard Wareing wrote:
> To better exercise the data path code of realtime subvolumes, we will
> set rtinherit=1 during mkfs calls.  For tests which this is not desired
> we introduce a _require_no_rtinherit function to opt out of this
> behavior.
> 
> Signed-off-by: Richard Wareing <rwareing@fb.com>
> ---
> Changes since v1:
> * None
> 
>  common/rc         | 24 +++++++++++++++++++++++-
>  tests/generic/250 |  1 +
>  tests/generic/252 |  1 +
>  tests/generic/427 |  1 +
>  tests/generic/441 |  1 +
>  tests/xfs/019     |  1 +
>  tests/xfs/031     |  1 +
>  tests/xfs/170     |  1 +
>  tests/xfs/187     |  1 +
>  9 files changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/common/rc b/common/rc
> index a0081f1..feed17f 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -33,6 +33,16 @@ BC=$(which bc 2> /dev/null) || BC=
>  VALID_TEST_ID="[0-9]\{3\}"
>  VALID_TEST_NAME="$VALID_TEST_ID-\?[[:alnum:]-]*"
>  
> +# When running tests with a realtime device configured, the realtime inherit
> +# flag will be set during mkfs via -d rtinherit=1 option.  For some tests
> +# this may render the test invalid (i.e. it uses a function which is not
> +# supported by the realtime subvolume); to prevent failure these tests may
> +# disable this behavior by calling _require_no_rtinherit .
> +_require_no_rtinherit()
> +{
> +	RT_INHERIT=false
> +}
> +
>  _require_math()
>  {
>  	if [ -z "$BC" ]; then
> @@ -562,6 +572,13 @@ _scratch_do_mkfs()
>  	local mkfs_status
>  	local tmp=`mktemp -u`
>  
> +	# Add rtinherit=1 to mkfs so we exercise realtime subvolume during
> +	# our tests.  Tests can opts out of this behavior by calling
> +	# _require_no_rtinherit.
> +	if $RT_INHERIT && echo "$mkfs_cmd" | grep rtdev &> /dev/null; then

Doesn't this if test fail if RT_INHERIT isn't defined, and isn't
RT_INHERIT undefined unless _reuqire_no_rtinherit?

Also, what happens if I forget the syntax and run 'RT_INHERIT=1 ./check'?
Deosn't that just spit out errors everywhere because '1' isn't a
command?

IOWs I was expecting some kind of string test, like

if [ "${RT_INHERIT}" != "false" ] && echo "${mkfs_cmd}" | grep -q rtdev;
then

--D

> +		extra_mkfs_options="$extra_mkfs_options -d rtinherit=1"
> +	fi
> +
>  	# save mkfs output in case conflict means we need to run again.
>  	# only the output for the mkfs that applies should be shown
>  	eval "$mkfs_cmd $MKFS_OPTIONS $extra_mkfs_options $SCRATCH_DEV" \
> @@ -760,7 +777,12 @@ _mkfs_dev()
>  	;;
>  
>      *)
> -	yes | $MKFS_PROG -t $FSTYP -- $MKFS_OPTIONS $* \
> +	local extra_mkfs_options="$*"
> +	# Similar behavior to the scratch variant of this
> +	if $RT_INHERIT && echo $extra_mkfs_options | grep rtdev &> /dev/null; then
> +		extra_mkfs_options="$extra_mkfs_options -d rtinherit=1"
> +	fi
> +	yes | $MKFS_PROG -t $FSTYP -- $MKFS_OPTIONS $extra_mkfs_options \
>  		2>$tmp.mkfserr 1>$tmp.mkfsstd
>  	;;
>      esac
> diff --git a/tests/generic/250 b/tests/generic/250
> index 3c4fe6d..9f4e364 100755
> --- a/tests/generic/250
> +++ b/tests/generic/250
> @@ -48,6 +48,7 @@ _require_scratch
>  _require_dm_target error
>  _require_xfs_io_command "falloc"
>  _require_odirect
> +_require_no_rtinherit
>  
>  rm -f $seqres.full
>  
> diff --git a/tests/generic/252 b/tests/generic/252
> index ffedd56..1156902 100755
> --- a/tests/generic/252
> +++ b/tests/generic/252
> @@ -47,6 +47,7 @@ _supported_os Linux
>  _require_scratch
>  _require_dm_target error
>  _require_xfs_io_command "falloc"
> +_require_no_rtinherit
>  _require_aiodio "aiocp"
>  AIO_TEST="src/aio-dio-regress/aiocp"
>  
> diff --git a/tests/generic/427 b/tests/generic/427
> index 9cde5f5..18f8476 100755
> --- a/tests/generic/427
> +++ b/tests/generic/427
> @@ -53,6 +53,7 @@ _supported_os Linux
>  _require_scratch
>  _require_test_program "feature"
>  _require_aiodio aio-dio-eof-race
> +_require_no_rtinherit
>  
>  # limit the filesystem size, to save the time of filling filesystem
>  _scratch_mkfs_sized $((256 * 1024 * 1024)) >>$seqres.full 2>&1
> diff --git a/tests/generic/441 b/tests/generic/441
> index 075d877..589069a 100755
> --- a/tests/generic/441
> +++ b/tests/generic/441
> @@ -47,6 +47,7 @@ _cleanup()
>  # real QA test starts here
>  _supported_os Linux
>  _require_scratch
> +_require_no_rtinherit
>  
>  # Generally, we want to avoid journal errors on the extended testcase. Only
>  # unset the -s flag if we have a logdev
> diff --git a/tests/xfs/019 b/tests/xfs/019
> index 3e4f169..1ab8991 100755
> --- a/tests/xfs/019
> +++ b/tests/xfs/019
> @@ -66,6 +66,7 @@ _supported_fs xfs
>  _supported_os Linux
>  
>  _require_scratch
> +_require_no_rtinherit
>  
>  protofile=$tmp.proto
>  tempfile=$tmp.file
> diff --git a/tests/xfs/031 b/tests/xfs/031
> index b05f28b..321f67a 100755
> --- a/tests/xfs/031
> +++ b/tests/xfs/031
> @@ -96,6 +96,7 @@ _supported_os Linux
>  
>  _require_scratch
>  _require_no_large_scratch_dev
> +_require_no_rtinherit
>  
>  # sanity test - default + one root directory entry
>  # Note: must do this proto/mkfs now for later inode size calcs
> diff --git a/tests/xfs/170 b/tests/xfs/170
> index c5ae8e4..6deef1b 100755
> --- a/tests/xfs/170
> +++ b/tests/xfs/170
> @@ -50,6 +50,7 @@ _supported_fs xfs
>  _supported_os Linux
>  
>  _require_scratch
> +_require_no_rtinherit
>  
>  _check_filestreams_support || _notrun "filestreams not available"
>  
> diff --git a/tests/xfs/187 b/tests/xfs/187
> index 07ef3ae..89e7b11 100755
> --- a/tests/xfs/187
> +++ b/tests/xfs/187
> @@ -56,6 +56,7 @@ _filter_version()
>  _supported_fs xfs
>  _supported_os Linux
>  
> +_require_no_rtinherit
>  _require_scratch
>  _require_attrs
>  _require_attr_v1
> -- 
> 2.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html