Re: [PATCH 2/3] modify xfs/ quota tests to work on generic filesystems

[Eric Sandeen <sandeen@sandeen.net>]

On 9/21/16 4:37 AM, Eryu Guan wrote:
> On Tue, Sep 20, 2016 at 05:21:46PM -0500, Eric Sandeen wrote:
>> Fix several xfs quota tests to work on non-xfs filesystems.
>>
>> New _require function to be sure that the xfs_quota tool can operate
>> on foreign filesystems; if it can, and if it needs to, it adds "-f"
>> to the XFS_QUOTA_PROG variable.
>>
>> Modify _qmount to do quotacheck/quotaon to mount and /enable/
>> quota.  On xfs this isn't needed and/or fails; it's ignored.
>>
>> All quota-related options used as arguments to _qmount are changed
>> from i.e. uquota to i.e. usrquota; the latter is standard across
>> almost all linux filesytems, including xfs.
>>
>> xfs/260 filters out the root/default quota line, because ext4
>> consumes a different amount of space than xfs, and it's not what
>> we're testing for, so just ignore it.
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>>
>> diff --git a/common/quota b/common/quota
>> index d91062a..2ec56a5 100644
>> --- a/common/quota
>> +++ b/common/quota
>> @@ -78,6 +78,19 @@ _require_xfs_quota()
>>  }
>>  
>>  #
>> +# checks that xfs_quota can operate on foreign (non-xfs) filesystems
>> +# Skips check on xfs filesystems, old xfs_quota is fine there.
>> +# Appends "-f" to enable foreign behavior on non-xfs filesystems if available.
>> +#
>> +_require_xfs_quota_foreign()
>> +{
>> +    if [ "$FSTYP" != "xfs" ]; then
>> +	$XFS_QUOTA_PROG -f -V &>/dev/null || \
>> +		_notrun "xfs_quota binary does not support foreign filesystems"
>> +	XFS_QUOTA_PROG="$XFS_QUOTA_PROG -f"
>> +    fi
> 
> Mixing space and tab in this function.

yep -

As do the functions before and after it - are we going with a strict
rule now or going for consistency with the rest of the code?

If you really want a strict tabs-only rule I'd suggest re-tabbing the
entire file to avoid this coming up over and over.  :)

>> +}
> 
> Better to have an empty line here :)

Yep :)

>> +#
>>  # checks that the XFS project quota support in the kernel is enabled.
>>  #
>>  _require_prjquota()
> 
> [snip]
> 
>> diff --git a/tests/xfs/118 b/tests/xfs/118
>> index 8414594..da18a32 100755
>> --- a/tests/xfs/118
>> +++ b/tests/xfs/118
>> @@ -47,11 +47,12 @@ _cleanup()
>>  # real QA test starts here
>>  
>>  # Modify as appropriate.
>> -_supported_fs xfs
>> +_supported_fs generic
>>  _supported_os IRIX Linux
>>  
>>  _require_scratch
>> -_require_xfs_quota
>> +_require_quota
>> +_require_xfs_quota_foreign
>>  
>>  _chowning_file()
>>  {
>> @@ -72,10 +73,10 @@ _chowning_file()
>>  }
>>  
>>  echo "mkfs on scratch"
>> -_scratch_mkfs_xfs >$seqres.full
>> +_scratch_mkfs >$seqres.full
> 
> I found that stderr should be redirected to $seqres.full too, otherwise
> it fails with ext2/3 as
> 
> +mke2fs 1.42.9 (28-Dec-2013)
> 
> I can fix them at commit time if there's no other review comments.

hm, ok thanks.  Not sure how I missed that, unless newer e2fsprogs
doesn't go to stderr (I was testing with 1.43.x)

Thanks,
-Eric

> Thanks,
> Eryu
>