xfs
[Top] [All Lists]

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

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 1/3] xfstests: introduce _require_xfs_crc_sb pre-checkup routine
From: Jeff Liu <jeff.liu@xxxxxxxxxx>
Date: Tue, 03 Dec 2013 15:48:50 +0800
Cc: "xfs@xxxxxxxxxxx" <xfs@xxxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20131203005619.GL10988@dastard>
References: <52983C41.8000004@xxxxxxxxxx> <20131203003759.GI10988@dastard> <20131203005619.GL10988@dastard>
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.1.0
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@xxxxxxxxxx>
>>>
>>> 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@xxxxxxxxxx>
>>> ---
>>>  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

<Prev in Thread] Current Thread [Next in Thread>