Re: [PATCH v5 2/3] common/filter: Factor out expected XFS warnings for assert

Eryu Guan
2018-01-19T02:51:55Z

On Thu, Jan 18, 2018 at 10:29:15AM -0800, Darrick J. Wong wrote:
> On Thu, Jan 18, 2018 at 06:49:54PM +0800, xiao yang wrote:
> > 1) Introduce _require_no_xfs_bug_on_assert helper to check if XFS is built
> > with CONFIG_XFS_ASSERT_FATAL, and just call _require_no_xfs_debug if
> > /sys/fs/xfs/debug/bug_on_assert is not available.
> >
> > 2) Apply _require_no_xfs_bug_on_assert in xfs/098 and xfs/115.
> >
> > 3) Move filter_xfs_dmesg from xfs/098 to common/filter, and rename
> > it as _filter_assert_dmesg.
> >
> > Signed-off-by: xiao yang <yangx.jy@cn.fujitsu.com>
> > ---
> > common/filter | 11 +++++++++++
> > common/xfs | 12 ++++++++++++
> > tests/xfs/098 | 20 ++++++++------------
> > tests/xfs/115 | 7 ++++---
> > 4 files changed, 35 insertions(+), 15 deletions(-)
> >
> > diff --git a/common/filter b/common/filter
> > index 8e1fdb4..53874a0 100644
> > --- a/common/filter
> > +++ b/common/filter
> > @@ -570,5 +570,16 @@ _filter_aiodio_dmesg()
> > -e "s#$warn9#Intentional warnings in dio_complete#"
> > }
> >
> > +# We generate assert related WARNINGs on purpose and make sure test doesn't fail
> > +# because of these warnings. This is a helper for _check_dmesg() to filter out
> > +# them.
> > +_filter_assert_dmesg()
> > +{
> > + local warn1="WARNING:.*fs/xfs/xfs_message\.c:.*asswarn.*"
> > + local warn2="WARNING:.*fs/xfs/xfs_message\.c:.*assfail.*"
> > + sed -e "s#$warn1#Intentional warnings in asswarn#" \
> > + -e "s#$warn2#Intentional warnings in assfail#"
> > +}
> > +
> > # make sure this script returns success
> > /bin/true
> > diff --git a/common/xfs b/common/xfs
> > index ab58364..a0e03b6 100644
> > --- a/common/xfs
> > +++ b/common/xfs
> > @@ -625,6 +625,18 @@ _require_no_xfs_debug()
> > fi
> > }
> >
> > +# Check if XFS is built with CONFIG_XFS_ASSERT_FATAL
> > +_require_no_xfs_bug_on_assert()
> > +{
> > + if [ -f /sys/fs/xfs/debug/bug_on_assert ]; then
> > + grep -q "1" /sys/fs/xfs/debug/bug_on_assert && \
> > + _notrun "Require XFS built without CONFIG_XFS_ASSERT_FATAL"
>
> This knob can be written, so perhaps we'd rather just set it to zero
> instead of _notrun?

I thought about it too, in the end I think it's better to not change the
behavior in test, this simplies the code a bit (if we change it, do we
need to change it back after test?) and gives the testers full control
on assert behavior (tester would want a crash in debugging?).

Perhaps we could update the _notrun message a bit to indicate that
bug_on_assert could be turned off if you want test to run.

Thanks,
Eryu