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

[Eryu Guan <eguan@redhat.com>]

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