[PATCH v3] generic/508: fix to check inode creation time feature on scratch mountpoint

[PATCH v3] generic/508: fix to check inode creation time feature on scratch mountpoint

[Chao Yu]

_require_btime() just check inode creation time feature on TEST_DIR
mountpoint, but generic/508 needs to do that check on SCRATCH_MNT
mountpoint. Let's add _require_scratch_btime() for that, meanwhile
handling scratch_{mkfs,mount,umount} inside the function to decouple
with caller.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
v3:
- hanlde scratch mkfs/mount/umount inside _require_scratch_btime
- relocate _require_scratch_btime
 common/rc         | 14 ++++++++++++++
 tests/generic/508 |  2 +-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/common/rc b/common/rc
index b4987a9cd7f7..a882fada7d02 100644
--- a/common/rc
+++ b/common/rc
@@ -3851,6 +3851,20 @@ _require_btime()
 	rm -f $TEST_DIR/test_creation_time
 }
 
+_require_scratch_btime()
+{
+	_require_scratch
+	_scratch_mkfs > /dev/null 2>&1
+	_scratch_mount
+
+	$XFS_IO_PROG -f $SCRATCH_MNT/test_creation_time -c "statx -v" \
+		| grep btime >>$seqres.full 2>&1 || \
+		_notrun "inode creation time not supported by this filesystem"
+	rm -f $SCRATCH_MNT/test_creation_time
+
+	_scratch_unmount
+}
+
 init_rc
 
 ################################################################################
diff --git a/tests/generic/508 b/tests/generic/508
index b869b3a9c260..3c2d4aa11169 100755
--- a/tests/generic/508
+++ b/tests/generic/508
@@ -46,10 +46,10 @@ _supported_os Linux
 _require_test_lsattr
 _require_statx
 _require_xfs_io_command "statx" "-v"
-_require_btime
 
 _require_scratch
 _require_scratch_shutdown
+_require_scratch_btime
 
 _scratch_mkfs >/dev/null 2>&1
 _require_metadata_journaling $SCRATCH_DEV
-- 
2.18.0.rc1

Re: [PATCH v3] generic/508: fix to check inode creation time feature on scratch mountpoint

[Dave Chinner]

On Sat, Oct 27, 2018 at 09:55:20AM +0800, Chao Yu wrote:
> _require_btime() just check inode creation time feature on TEST_DIR
> mountpoint, but generic/508 needs to do that check on SCRATCH_MNT
> mountpoint. Let's add _require_scratch_btime() for that, meanwhile
> handling scratch_{mkfs,mount,umount} inside the function to decouple
> with caller.
> 
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
> v3:
> - hanlde scratch mkfs/mount/umount inside _require_scratch_btime
> - relocate _require_scratch_btime
>  common/rc         | 14 ++++++++++++++
>  tests/generic/508 |  2 +-
>  2 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/common/rc b/common/rc
> index b4987a9cd7f7..a882fada7d02 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -3851,6 +3851,20 @@ _require_btime()
>  	rm -f $TEST_DIR/test_creation_time
>  }
>  
> +_require_scratch_btime()
> +{
> +	_require_scratch
> +	_scratch_mkfs > /dev/null 2>&1
> +	_scratch_mount
> +
> +	$XFS_IO_PROG -f $SCRATCH_MNT/test_creation_time -c "statx -v" \
> +		| grep btime >>$seqres.full 2>&1 || \
> +		_notrun "inode creation time not supported by this filesystem"
> +	rm -f $SCRATCH_MNT/test_creation_time

No need to remove that file - it's going to get mkfs'd again,
anyway.

Otherwise looks fine. I'm not a fan of all these require_scratch
rules having to run mkfs, but I can live with it.

Reviewed-by: Dave Chinner <dchinner@redhat.com>
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH v3] generic/508: fix to check inode creation time feature on scratch mountpoint

[Chao Yu]

On 2018/10/27 11:28, Dave Chinner wrote:
> On Sat, Oct 27, 2018 at 09:55:20AM +0800, Chao Yu wrote:
>> _require_btime() just check inode creation time feature on TEST_DIR
>> mountpoint, but generic/508 needs to do that check on SCRATCH_MNT
>> mountpoint. Let's add _require_scratch_btime() for that, meanwhile
>> handling scratch_{mkfs,mount,umount} inside the function to decouple
>> with caller.
>>
>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>> ---
>> v3:
>> - hanlde scratch mkfs/mount/umount inside _require_scratch_btime
>> - relocate _require_scratch_btime
>>  common/rc         | 14 ++++++++++++++
>>  tests/generic/508 |  2 +-
>>  2 files changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/common/rc b/common/rc
>> index b4987a9cd7f7..a882fada7d02 100644
>> --- a/common/rc
>> +++ b/common/rc
>> @@ -3851,6 +3851,20 @@ _require_btime()
>>  	rm -f $TEST_DIR/test_creation_time
>>  }
>>  
>> +_require_scratch_btime()
>> +{
>> +	_require_scratch
>> +	_scratch_mkfs > /dev/null 2>&1
>> +	_scratch_mount
>> +
>> +	$XFS_IO_PROG -f $SCRATCH_MNT/test_creation_time -c "statx -v" \
>> +		| grep btime >>$seqres.full 2>&1 || \
>> +		_notrun "inode creation time not supported by this filesystem"
>> +	rm -f $SCRATCH_MNT/test_creation_time
> 
> No need to remove that file - it's going to get mkfs'd again,
> anyway.

That's correct, I just saw Eryu has fixed this in upstream, so I think I
don't need to send v4...

> 
> Otherwise looks fine. I'm not a fan of all these require_scratch
> rules having to run mkfs, but I can live with it.
> 
> Reviewed-by: Dave Chinner <dchinner@redhat.com>

Thanks for the review. :)

Thanks,

>