Re: [PATCH fstests v2 3/3] generic/578: only run on filesystems that support FIEMAP

["Darrick J. Wong" <djwong@kernel.org>]

On Fri, Aug 25, 2023 at 10:57:20AM -0400, Jeff Layton wrote:
> On Fri, 2023-08-25 at 22:16 +0800, Zorro Lang wrote:
> > On Thu, Aug 24, 2023 at 01:28:26PM -0400, Jeff Layton wrote:
> > > On Thu, 2023-08-24 at 10:09 -0700, Darrick J. Wong wrote:
> > > > On Thu, Aug 24, 2023 at 12:44:19PM -0400, Jeff Layton wrote:
> > > > > Some filesystems (e.g. NFS) don't support FIEMAP. Limit generic/578 to
> > > > > filesystems that do.
> > > > > 
> > > > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > > > ---
> > > > >  common/rc         | 13 +++++++++++++
> > > > >  tests/generic/578 |  1 +
> > > > >  2 files changed, 14 insertions(+)
> > > > > 
> > > > > diff --git a/common/rc b/common/rc
> > > > > index 33e74d20c28b..98d27890f6f7 100644
> > > > > --- a/common/rc
> > > > > +++ b/common/rc
> > > > > @@ -3885,6 +3885,19 @@ _require_metadata_journaling()
> > > > >  	fi
> > > > >  }
> > > > >  
> > > > > +_require_fiemap()
> > > > > +{
> > > > > +	local testfile=$TEST_DIR/fiemaptest.$$
> > > > > +
> > > > > +	touch $testfile
> > > > > +	$XFS_IO_PROG -r -c "fiemap" $testfile 1>$testfile.out 2>&1
> > > > > +	if grep -q 'Operation not supported' $testfile.out; then
> > > > > +	  _notrun "FIEMAP is not supported by this filesystem"
> > > > > +	fi
> > > > > +
> > > > > +	rm -f $testfile $testfile.out
> > > > > +}
> > > > 
> > > > _require_xfs_io_command "fiemap" ?
> > > > 
> > > > 
> > > 
> > > Ok, I figured we'd probably do this test after testing for that
> > > separately, but you're correct that we do require it here.
> > > 
> > > If we add that, should we also do this, at least in all of the general
> > > tests?
> > > 
> > >     s/_require_xfs_io_command "fiemap"/_require_fiemap/
> > > 
> > > I think we end up excluding some of those tests on NFS for other
> > > reasons, but other filesystems that don't support fiemap might still try
> > > to run these tests.
> > 
> > We have lots of cases contains _require_xfs_io_command "fiemap", so I think
> > we can keep this "tradition", don't bring a new _require_fiemap for now,
> > so ...
> > 
> > >  
> > > > > +
> > > > >  _count_extents()
> > > > >  {
> > > > >  	$XFS_IO_PROG -r -c "fiemap" $1 | tail -n +2 | grep -v hole | wc -l
> > > > > diff --git a/tests/generic/578 b/tests/generic/578
> > > > > index b024f6ff90b4..903055b2ca58 100755
> > > > > --- a/tests/generic/578
> > > > > +++ b/tests/generic/578
> > > > > @@ -26,6 +26,7 @@ _require_test_program "mmap-write-concurrent"
> > > > >  _require_command "$FILEFRAG_PROG" filefrag
> > > > >  _require_test_reflink
> > > > >  _require_cp_reflink
> > > > > +_require_fiemap
> > 
> > _require_xfs_io_command "fiemap"
> > 
> 
> That's not sufficient -- there is already a call to that in this test.
> 
> _require_xfs_io_command just validates that the xfs_io binary has
> plumbing for that command (which just issues an ioctl to the file).
> Even if the binary has support, the underlying filesystem has to support
> the ioctl.
> 
> Many don't, so we need to test for that specifically.

It /does/ test the kernel support for fiemap...

_require_xfs_io_command()
{
...
	"fiemap")
		# If 'ranged' is passed as argument then we check to see if fiemap supports
		# ranged query params
		if echo "$param" | grep -q "ranged"; then
			param=$(echo "$param" | sed "s/ranged//")
			$XFS_IO_PROG -c "help fiemap" | grep -q "\[offset \[len\]\]"
			[ $? -eq 0 ] || _notrun "xfs_io $command ranged support is missing"
		fi
		testio=`$XFS_IO_PROG -F -f -c "pwrite 0 20k" -c "fsync" \
			-c "fiemap -v $param" $testfile 2>&1`
		param_checked="$param"
		;;

--D

> > > > >  
> > > > >  compare() {
> > > > >  	for i in $(seq 1 8); do
> > > > > 
> > > > > -- 
> > > > > 2.41.0
> > > > > 
> > > 
> > > -- 
> > > Jeff Layton <jlayton@kernel.org>
> > > 
> > 
> 
> -- 
> Jeff Layton <jlayton@kernel.org>