| Message ID | 20170925195625.756877-2-rwareing@fb.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
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 -- 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
Darrick J. Wong <darrick.wong@oracle.com> wrote: > 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? > My bad on this, I have RT_INHERIT defined in my config.local, which explains why it works so nicely for me :). So yes this will have to be changed to default to something sane or a test. > 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 -- 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
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(-) I don't think we need an extra RT_INHERT flag, this is just one of the normal test configurations and can be easily tested by setting correct MKFS_OPTIONS, e.g. MKFS_OPTIONS="-d rtinherit=1" ./check ... Then we can check if MKFS_OPTIONS has rtinherit defined in _require_no_rtinherit(), e.g. _require_no_rtinherit() { echo "$MKFS_OPTIONS" | egrep -q "rtinherit([^=]|=1)" && \ _notrun "<notrun message here>" } Unfortunately, mkfs.xfs doesn't print rtinherit status at mkfs time, otherwise we could check mkfs.xfs output directly. And xfs/019 xfs/031 passed even with "-d rtinherit=1", and xfs/187 already _notrun with proper message, so seems they don't need _require_no_rtinherit rule. generic/427 fails because xfs_io prints out its write log, after redirecting stdout of xfs_io to /dev/null, generic/427 passed for me too, the xfs_io write is meant to write a special pattern to the disk anyway. For other tests that do need _require_no_rtinherit, a comment to explain why this rule is needed would be good. Thanks, Eryu -- 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
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 + 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
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(-)