On Thu, Jun 29, 2017 at 07:54:09AM -0400, Brian Foster wrote:
> On Wed, Jun 28, 2017 at 09:40:00PM -0700, Darrick J. Wong wrote:
> > On Wed, Jun 28, 2017 at 07:44:50AM -0400, Brian Foster wrote:
> > > Update the test to use the newly available sysfs errortag injection
> > > knobs. The errortag knob has replaced the previous custom knob in
> > > the kernel. Create a local helper to support the old knob for
> > > backwards compatibility.
> > >
> > > Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx>
> > > ---
> > >
> > > This updates xfs/141 to use the new error injection tag sysfs knob that
> > > has replaced log_badcrc_factor in the latest XFS for-next tree.
> >
> > Hmmm... for xfstests, I was thinking of rewriting the common/inject helpers:
> >
> > # Requires that xfs_io inject command knows about this error type
> > _require_xfs_io_error_injection()
> > {
> > type="$1"
> >
> > shortdev=$(_short_dev $TEST_DEV)
> > tagfile=/sys/fs/xfs/$shortdev/errortag/$type
> > test -w $tagfile && return
> >
> > _require_error_injection
> >
> > if [ "$type" = "log_bad_crc" ]; then
> > tagfile=/sys/fs/xfs/$shortdev/log/log_badcrc_factor
> > test -w $tagfile && return
> > _notrun "Cannot find log_badcrc_factor"
> > fi
> >
> > # NOTE: We can't actually test error injection here because xfs
> > # hasn't always range checked the argument to xfs_errortag_add.
> > # We also don't want to trip an error before we're ready to deal
> > # with it.
> >
> > $XFS_IO_PROG -x -c 'inject' $TEST_DIR | grep -q "$type" || \
> > _notrun "XFS error injection $type unknown."
> > }
> >
> > # Inject an error into the scratch fs
> > _scratch_inject_error()
> > {
> > type="$1"
> > value="$2"
> >
> > shortdev=$(_short_dev $SCRATCH_DEV)
> > tagfile=/sys/fs/xfs/$shortdev/errortag/$type
> > if [ -w $tagfile ]; then
> > echo "$value" > $tagfile
> > return
> > fi
> >
> > if [ "$type" = "log_bad_crc" ]; then
> > tagfile=/sys/fs/xfs/$shortdev/log/log_badcrc_factor
> > if [ -w $tagfile ]; then
> > echo "$value" > $tagfile
> > return
> > fi
> > _notrun "cannot inject error $type value $value"
> > elif [ -z "$value" ] || [ "$value" = "default" ]; then
> > $XFS_IO_PROG -x -c "inject $type" $SCRATCH_MNT
> > else
> > _notrun "cannot inject error $type value $value"
> > fi
> > }
> >
> > ...that way we can just change xfs/141 to do:
> >
> > _require_xfs_io_error_injection "log_bad_crc"
> > ...
> > for i in $(seq 1 5); do
> > # Enable error injection. Use a random bad crc factor up to 100
> > # (increase this value to run fsstress longer).
> > factor=$((RANDOM % 100 + 1))
> > ...
> > _scratch_inject_error "log_bad_crc" "$factor"
> > ...
> > done
> >
> > Therefore we can move all the other error injection knob tests (looking
> > at you, drop_writes) to a standard helper function that will cover all
> > the errortag knobs we're moving to sysfs. Unfortunately we pay for that
> > uniformity by uglifying the helper functions to handle the old knobs,
> > but oh well.
> >
>
> That all looks pretty good to me. There's only a couple or so old knobs
> being killed off, so it doesn't seem like that big of a deal.
>
> Are you planning to send a patch for the above? If so, we can just drop
> this one (and I'll update the tail overwrite test as well).
Yes, I'll clean that up a little more and send it out.
> > I do wonder about the old xfs_io command, though -- while it's tempting
> > to rewrite it to do all the parsing required to get to sysfs, I'm not so
> > enthused about doing in C what can be done much more easily in bash.
> >
>
> Not sure I follow... are you contemplating rewriting 'xfs_io -c inject
> ...' to actually use sysfs rather than the ioctl()? That seems like it
> would get kind of ugly, at first thought. If we really wanted the
> ability to set the frequency through xfs_io, I think the right approach
> is probably to define a new ioctl() that takes a frequency param (but
> I'm not really convinced there is a need).
Just musing a little about refitting the xfs_io command, and convincing
myself not to do it. :)
--D
>
> Brian
>
> > --D
> >
> > >
> > > Brian
> > >
> > > tests/xfs/141 | 22 ++++++++++++++++++----
> > > 1 file changed, 18 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/tests/xfs/141 b/tests/xfs/141
> > > index 56ff14e..ea4f558 100755
> > > --- a/tests/xfs/141
> > > +++ b/tests/xfs/141
> > > @@ -43,6 +43,21 @@ _cleanup()
> > > wait > /dev/null 2>&1
> > > }
> > >
> > > +# require either the current error tag or the old custom log record crc error
> > > +# injection knob
> > > +_setup_bad_crc()
> > > +{
> > > + if [ -e /sys/fs/xfs/$1/errortag/log_bad_crc ]; then
> > > + badcrc_attr=/sys/fs/xfs/$1/errortag/log_bad_crc
> > > + return
> > > + fi
> > > + if [ -e /sys/fs/xfs/$1/log/log_badcrc_factor ]; then
> > > + badcrc_attr=/sys/fs/xfs/$1/log/log_badcrc_factor
> > > + return
> > > + fi
> > > + _notrun "bad crc sysfs attribute not supported"
> > > +}
> > > +
> > > rm -f $seqres.full
> > >
> > > # get standard environment, filters and checks
> > > @@ -53,7 +68,6 @@ rm -f $seqres.full
> > > # Modify as appropriate.
> > > _supported_fs xfs
> > > _supported_os Linux
> > > -_require_xfs_sysfs $(_short_dev $TEST_DEV)/log/log_badcrc_factor
> > > _require_scratch
> > > _require_command "$KILLALL_PROG" killall
> > >
> > > @@ -62,14 +76,14 @@ echo "Silence is golden."
> > > _scratch_mkfs >> $seqres.full 2>&1 || _fail "mkfs failed"
> > > _scratch_mount
> > >
> > > -sdev=$(_short_dev $SCRATCH_DEV)
> > > +_setup_bad_crc $(_short_dev $SCRATCH_DEV)
> > >
> > > for i in $(seq 1 5); do
> > > # Enable error injection. Use a random bad crc factor up to 100
> > > # (increase this value to run fsstress longer).
> > > factor=$((RANDOM % 100 + 1))
> > > - echo iteration $i log_badcrc_factor: $factor >> $seqres.full 2>&1
> > > - echo $factor > /sys/fs/xfs/$sdev/log/log_badcrc_factor
> > > + echo iteration $i bad crc factor: $factor >> $seqres.full 2>&1
> > > + echo $factor > $badcrc_attr
> > >
> > > # Run fsstress until the filesystem shuts down. It will shut down
> > > # automatically when error injection triggers.
> > > --
> > > 2.9.4
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe fstests" in
> > > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > --
> > To unsubscribe from this list: send the line "unsubscribe fstests" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > 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@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe fstests" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
