Re: [PATCH] generic/038: require fallocate support and test for trim support

Re: [PATCH] generic/038: require fallocate support and test for trim support

[Dave Chinner]

On Mon, Dec 15, 2014 at 11:26:35PM +0100, Dushan Tcholich wrote:
> Add tests for allocate support and test if TRIM really works on tested 
> partition.
> 
> Signed-off-by: Dushan Tcholich <dusanc@gmail.com>
> 
> --- xfstests.orig/tests/generic/038	2014-12-14 15:18:00.000000000 +0100
> +++ xfstests/tests/generic/038	2014-12-15 23:21:11.000000000 +0100
> @@ -70,6 +70,10 @@
>  _supported_os Linux
>  _require_scratch
>  _require_fstrim
> +_require_xfs_io_command "falloc"
> +_require_xfs_io_command "truncate"

No need to test for the truncate command as it's supported in all
versions of xfs_io people use. falloc, OTOH, isn't supported on oler
distros that people still need to run QA on, and hence that check is
required....

> +_test_batched_discard $SCRATCH_MNT || _notrun "FITRIM not supported on $SCRATCH_DEV"

$SCRATCH_MNT is not mounted at this point. The test needs to go
after _scratch_mkfs/_scratch_mount...

Hmmm - I'm thinking this should really be a "_requires_batched_discard"
function. It already internally calls _require_fstrim, so we should
just drive everything inwards. i.e.:

_require_batched_discard()
{
	if [ $# -ne 1 ]; then
		echo "Usage: _require_batched_discard mnt_point" 1>&2
		exit 1
	fi
	_require_fstrim
	[ $FSTRIM_PROG $1 > /dev/null 2>&1 ] || \
		_notrun "FITRIM not supported on $1"
}

And so not even need the _requires_fstrim call in these tests...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] generic/038: require fallocate support and test for trim support

[Theodore Ts'o]

On Tue, Dec 16, 2014 at 07:49:15AM +1100, Dave Chinner wrote:
> On Mon, Dec 15, 2014 at 11:26:35PM +0100, Dushan Tcholich wrote:
> > Add tests for allocate support and test if TRIM really works on tested 
> > partition.
> > 
> > Signed-off-by: Dushan Tcholich <dusanc@gmail.com>
> > 
> > --- xfstests.orig/tests/generic/038	2014-12-14 15:18:00.000000000 +0100
> > +++ xfstests/tests/generic/038	2014-12-15 23:21:11.000000000 +0100
> > @@ -70,6 +70,10 @@
> >  _supported_os Linux
> >  _require_scratch
> >  _require_fstrim
> > +_require_xfs_io_command "falloc"
> > +_require_xfs_io_command "truncate"
> 
> No need to test for the truncate command as it's supported in all
> versions of xfs_io people use. falloc, OTOH, isn't supported on oler
> distros that people still need to run QA on, and hence that check is
> required....

I've also been using '_require_xfs_io_command "falloc"' to test
whether the file system supports fallocate(2).  So for example, in the
patch that I sent out today, I'm checking not just whether xfs_io
supports "falloc", but whether the file system under test (at least
with a specific configuration, such as ext4 in ext3 compatibility
mode) supports fallocate(2).  Do you consider that a valid thing to
do?

I could propose creating a separate _require_fallocate macro, but it
would basically be doing the equivalent thing to
_require_xfs_io_command "falloc", and in the tests that I was looking
at, we were using xfs_io and falloc anyway.  So the point is somewhat
moot, but if and when we stop suppotring RHEL 3, or whatever
enterprise distro was driving this check, it would be nice if we could
keep those checks around.

       	  	   	      	    	 - Ted

Re: [PATCH] generic/038: require fallocate support and test for trim support

[Dave Chinner]

On Mon, Dec 15, 2014 at 04:45:10PM -0500, Theodore Ts'o wrote:
> On Tue, Dec 16, 2014 at 07:49:15AM +1100, Dave Chinner wrote:
> > On Mon, Dec 15, 2014 at 11:26:35PM +0100, Dushan Tcholich wrote:
> > > Add tests for allocate support and test if TRIM really works on tested 
> > > partition.
> > > 
> > > Signed-off-by: Dushan Tcholich <dusanc@gmail.com>
> > > 
> > > --- xfstests.orig/tests/generic/038	2014-12-14 15:18:00.000000000 +0100
> > > +++ xfstests/tests/generic/038	2014-12-15 23:21:11.000000000 +0100
> > > @@ -70,6 +70,10 @@
> > >  _supported_os Linux
> > >  _require_scratch
> > >  _require_fstrim
> > > +_require_xfs_io_command "falloc"
> > > +_require_xfs_io_command "truncate"
> > 
> > No need to test for the truncate command as it's supported in all
> > versions of xfs_io people use. falloc, OTOH, isn't supported on oler
> > distros that people still need to run QA on, and hence that check is
> > required....
> 
> I've also been using '_require_xfs_io_command "falloc"' to test
> whether the file system supports fallocate(2).  So for example, in the
> patch that I sent out today, I'm checking not just whether xfs_io
> supports "falloc", but whether the file system under test (at least
> with a specific configuration, such as ext4 in ext3 compatibility
> mode) supports fallocate(2).  Do you consider that a valid thing to
> do?

Yes, that's it's intent. From the 2009 patch that introduced
checks for fallocate support:

+# check that xfs_io, glibc, kernel, and filesystem all (!) support
+# fallocate
+#
+_require_xfs_io_falloc()

i.e. the one function checks the entire stack for fallocate support.
That's not going to change.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

[PATCH] generic/038: require fallocate support and test for trim support

[Dushan Tcholich]

Add tests for allocate support and test if TRIM really works on tested 
partition.

Signed-off-by: Dushan Tcholich <dusanc@gmail.com>

--- xfstests.orig/tests/generic/038	2014-12-14 15:18:00.000000000 +0100
+++ xfstests/tests/generic/038	2014-12-15 23:21:11.000000000 +0100
@@ -70,6 +70,10 @@
 _supported_os Linux
 _require_scratch
 _require_fstrim
+_require_xfs_io_command "falloc"
+_require_xfs_io_command "truncate"
+
+_test_batched_discard $SCRATCH_MNT || _notrun "FITRIM not supported on $SCRATCH_DEV"
 
 rm -f $seqres.full