[PATCH 1/3] xfstests: introduce _require_xfs_crc_sb pre-checkup routine

[PATCH 1/3] xfstests: introduce _require_xfs_crc_sb pre-checkup routine

[Jeff Liu]

From: Jie Liu <jeff.liu@oracle.com>

Introduce _require_xfs_crc_sb to rc.  We can use it in pre-checkup
procedures to verify if the XFS test environment is configured with
CRC enabled or not for some particular test cases.

Introduce a new helper _xfs_sb_version to get XFS super block version
to make the test case more flexiable if possible. e.g, if that is
v5 super block, the tests behavior might be different to old v4 sb.

Refactor xfs/299 to use it.

Signed-off-by: Jie Liu <jeff.liu@oracle.com>
---
 common/rc     | 19 +++++++++++++++++++
 tests/xfs/299 |  9 +--------
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/common/rc b/common/rc
index f73414b..9f45279 100644
--- a/common/rc
+++ b/common/rc
@@ -1579,6 +1579,25 @@ _check_xfs_filesystem()
     return 0
 }
 
+# Get XFS super block version
+_xfs_sb_version()
+{
+	sb_version=`$XFS_DB_PROG -r -c version $SCRATCH_DEV | \
+		    awk -F= '{print $2}' | awk -F, '{print $1}' | \
+		    cut -c3-`
+	echo $sb_version
+}
+
+# This test requires XFS crc-enabled (v5) super block support
+_require_xfs_crc_sb()
+{
+	version=`_xfs_sb_version`
+	if [ $version -lt 5 ]
+	then
+		_notrun "Can't run with older versions of superblock"
+	fi
+}
+
 # Filter the knowen errors the UDF Verifier reports.
 _udf_test_known_error_filter()
 {
diff --git a/tests/xfs/299 b/tests/xfs/299
index d5e8c5e..3c42908 100755
--- a/tests/xfs/299
+++ b/tests/xfs/299
@@ -54,6 +54,7 @@ chmod a+rwx $seqres.full	# arbitrary users will write here
 
 _require_scratch
 _require_xfs_quota
+_require_xfs_crc_sb
 
 bsoft=100
 bhard=500
@@ -198,14 +199,6 @@ cat $tmp.mkfs >>$seqres.full
 # keep the blocksize and data size for dd later
 . $tmp.mkfs
 
-sb_ver=`$XFS_DB_PROG -r -c version $SCRATCH_DEV | awk -F= '{print $2}' | awk -F, '{print $1}' |cut -c3-`
-echo $sb_ver
-
-if [ $sb_ver -lt 5 ]
-then
-	_notrun "Can't run with older versions of superblock"
-fi
-
 cat >$tmp.projects <<EOF
 1:$SCRATCH_MNT
 EOF
-- 
1.8.3.2

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/3] xfstests: introduce _require_xfs_crc_sb pre-checkup routine

[Dave Chinner]

On Fri, Nov 29, 2013 at 03:03:29PM +0800, Jeff Liu wrote:
> From: Jie Liu <jeff.liu@oracle.com>
> 
> Introduce _require_xfs_crc_sb to rc.  We can use it in pre-checkup
> procedures to verify if the XFS test environment is configured with
> CRC enabled or not for some particular test cases.
> 
> Introduce a new helper _xfs_sb_version to get XFS super block version
> to make the test case more flexiable if possible. e.g, if that is
> v5 super block, the tests behavior might be different to old v4 sb.
> 
> Refactor xfs/299 to use it.
> 
> Signed-off-by: Jie Liu <jeff.liu@oracle.com>
> ---
>  common/rc     | 19 +++++++++++++++++++
>  tests/xfs/299 |  9 +--------
>  2 files changed, 20 insertions(+), 8 deletions(-)
> 
> diff --git a/common/rc b/common/rc
> index f73414b..9f45279 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -1579,6 +1579,25 @@ _check_xfs_filesystem()
>      return 0
>  }
>  
> +# Get XFS super block version
> +_xfs_sb_version()
> +{
> +	sb_version=`$XFS_DB_PROG -r -c version $SCRATCH_DEV | \
> +		    awk -F= '{print $2}' | awk -F, '{print $1}' | \
> +		    cut -c3-`
> +	echo $sb_version
> +}

Ugh. I missed how nasty that code was originally. Maths, not string
manipulations shoul dbe used here. i.e:

	versionnum  filesystem  version  information.   This value is
		    currently 1, 2, 3, or 4 in the low 4 bits.

[ Oh, look, xfs_db man page updates are needed ]

So:

	$XFS_DB_PROG -r -c "sb 0" -c "p versionnum" $SCRATCH_DEV | \
		awk '{ printf "%d\n", and(strtonum($3), 15) }'


> +# This test requires XFS crc-enabled (v5) super block support
> +_require_xfs_crc_sb()
> +{
> +	version=`_xfs_sb_version`
> +	if [ $version -lt 5 ]
> +	then
> +		_notrun "Can't run with older versions of superblock"

		_notrun "Need version 5 superblock support for this test"
> +	fi
> +}
> +

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/3] xfstests: introduce _require_xfs_crc_sb pre-checkup routine

[Dave Chinner]

On Tue, Dec 03, 2013 at 11:37:59AM +1100, Dave Chinner wrote:
> On Fri, Nov 29, 2013 at 03:03:29PM +0800, Jeff Liu wrote:
> > From: Jie Liu <jeff.liu@oracle.com>
> > 
> > Introduce _require_xfs_crc_sb to rc.  We can use it in pre-checkup
> > procedures to verify if the XFS test environment is configured with
> > CRC enabled or not for some particular test cases.
> > 
> > Introduce a new helper _xfs_sb_version to get XFS super block version
> > to make the test case more flexiable if possible. e.g, if that is
> > v5 super block, the tests behavior might be different to old v4 sb.
> > 
> > Refactor xfs/299 to use it.
> > 
> > Signed-off-by: Jie Liu <jeff.liu@oracle.com>
> > ---
> >  common/rc     | 19 +++++++++++++++++++
> >  tests/xfs/299 |  9 +--------
> >  2 files changed, 20 insertions(+), 8 deletions(-)
> > 
> > diff --git a/common/rc b/common/rc
> > index f73414b..9f45279 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -1579,6 +1579,25 @@ _check_xfs_filesystem()
> >      return 0
> >  }
> >  
> > +# Get XFS super block version
> > +_xfs_sb_version()
> > +{
> > +	sb_version=`$XFS_DB_PROG -r -c version $SCRATCH_DEV | \
> > +		    awk -F= '{print $2}' | awk -F, '{print $1}' | \
> > +		    cut -c3-`
> > +	echo $sb_version
> > +}
> 
> Ugh. I missed how nasty that code was originally. Maths, not string
> manipulations shoul dbe used here. i.e:
> 
> 	versionnum  filesystem  version  information.   This value is
> 		    currently 1, 2, 3, or 4 in the low 4 bits.
> 
> [ Oh, look, xfs_db man page updates are needed ]
> 
> So:
> 
> 	$XFS_DB_PROG -r -c "sb 0" -c "p versionnum" $SCRATCH_DEV | \
> 		awk '{ printf "%d\n", and(strtonum($3), 15) }'

I just realised that the way this is being used is invalid. WE need
to check that mkfs supports -m crc=1, and that the filesystem that
is created can be mounted by the kernel.

i.e. You can't just look at the
superblock version number on the scratch device
and say that the test can run because:

	a) the scratch device has not been initialised by the test
	   and so can contain garbage from previous tests; and
	b) the kernel might not support CRCs even though the
	   userspace utilities do.

Hence we need to test for both userspace and kernel support here.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/3] xfstests: introduce _require_xfs_crc_sb pre-checkup routine

[Jeff Liu]

On 12/03 2013 08:56 AM, Dave Chinner wrote:
> On Tue, Dec 03, 2013 at 11:37:59AM +1100, Dave Chinner wrote:
>> On Fri, Nov 29, 2013 at 03:03:29PM +0800, Jeff Liu wrote:
>>> From: Jie Liu <jeff.liu@oracle.com>
>>>
>>> Introduce _require_xfs_crc_sb to rc.  We can use it in pre-checkup
>>> procedures to verify if the XFS test environment is configured with
>>> CRC enabled or not for some particular test cases.
>>>
>>> Introduce a new helper _xfs_sb_version to get XFS super block version
>>> to make the test case more flexiable if possible. e.g, if that is
>>> v5 super block, the tests behavior might be different to old v4 sb.
>>>
>>> Refactor xfs/299 to use it.
>>>
>>> Signed-off-by: Jie Liu <jeff.liu@oracle.com>
>>> ---
>>>  common/rc     | 19 +++++++++++++++++++
>>>  tests/xfs/299 |  9 +--------
>>>  2 files changed, 20 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/common/rc b/common/rc
>>> index f73414b..9f45279 100644
>>> --- a/common/rc
>>> +++ b/common/rc
>>> @@ -1579,6 +1579,25 @@ _check_xfs_filesystem()
>>>      return 0
>>>  }
>>>  
>>> +# Get XFS super block version
>>> +_xfs_sb_version()
>>> +{
>>> +	sb_version=`$XFS_DB_PROG -r -c version $SCRATCH_DEV | \
>>> +		    awk -F= '{print $2}' | awk -F, '{print $1}' | \
>>> +		    cut -c3-`
>>> +	echo $sb_version
>>> +}
>>
>> Ugh. I missed how nasty that code was originally. Maths, not string
>> manipulations shoul dbe used here. i.e:
>>
>> 	versionnum  filesystem  version  information.   This value is
>> 		    currently 1, 2, 3, or 4 in the low 4 bits.
>>
>> [ Oh, look, xfs_db man page updates are needed ]
>>
>> So:
>>
>> 	$XFS_DB_PROG -r -c "sb 0" -c "p versionnum" $SCRATCH_DEV | \
>> 		awk '{ printf "%d\n", and(strtonum($3), 15) }'
> 
> I just realised that the way this is being used is invalid. WE need
> to check that mkfs supports -m crc=1, and that the filesystem that
> is created can be mounted by the kernel.
> 
> i.e. You can't just look at the
> superblock version number on the scratch device
> and say that the test can run because:
> 
> 	a) the scratch device has not been initialised by the test
> 	   and so can contain garbage from previous tests; and
> 	b) the kernel might not support CRCs even though the
> 	   userspace utilities do.
> 
> Hence we need to test for both userspace and kernel support here.
Definitely, I'll add those pre-checkups and fix 299 accordingly.

Thanks,
-Jeff

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs