Re: [PATCH 2/4] common/xfs: Add helper to check if nrext64 option is supported

[Chandan Babu R <chandan.babu@oracle.com>]

On Wed, Jun 08, 2022 at 09:01:50 AM +1000, Dave Chinner wrote:
> On Mon, Jun 06, 2022 at 06:10:59PM +0530, Chandan Babu R wrote:
>> This commit adds a new helper to allow tests to check if xfsprogs and xfs
>> kernel module support nrext64 option.
>> 
>> Signed-off-by: Chandan Babu R <chandan.babu@oracle.com>
>> ---
>>  common/xfs | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>> 
>> diff --git a/common/xfs b/common/xfs
>> index 2123a4ab..dca7af57 100644
>> --- a/common/xfs
>> +++ b/common/xfs
>> @@ -1328,3 +1328,16 @@ _xfs_filter_mkfs()
>>  		print STDOUT "realtime =RDEV extsz=XXX blocks=XXX, rtextents=XXX\n";
>>  	}'
>>  }
>> +
>> +_require_scratch_xfs_nrext64()
>> +{
>> +	_require_scratch
>
> Not needed - caller should be doing that first.
>
>> +
>> +	_scratch_mkfs -i nrext64=1 &>/dev/null || \
>> +		_notrun "mkfs.xfs doesn't support nrext64 feature"
>
> _scratch_mkfs_xfs_supported -i nrext64=1
>
> see for example:
>
> _require_xfs_mkfs_crc
>
>> +	_try_scratch_mount || \
>> +		_notrun "kernel doesn't support xfs nrext64 feature"
>> +	$XFS_INFO_PROG "$SCRATCH_MNT" | grep -q -w "nrext64=1" || \
>> +		_notrun "nrext64 feature not advertised on mount?"
>
> This seems unnecessary - if mkfs supports the feature bit, and the
> kernel supports is, this should just work. We don't do checks like
> this in any other feature bit test. e.g:
>
> _require_xfs_finobt
> _require_xfs_sparse_inodes
>
> etc.
>
> Also, you should put this in the same region of the file as all the
> other feature checks, not right down the bottom by itself.
>

I agree. I will make the relevant changes and post a V2.

-- 
chandan