[PATCH v2] common/casefold: Support for tmpfs casefold test

[PATCH v2] common/casefold: Support for tmpfs casefold test

[André Almeida]

Test casefold support for tmpfs.

Signed-off-by: André Almeida <andrealmeid@igalia.com>
---
The kernel side was accepted and can be currently found at vfs.all tree.

This is the expected test output:

$ sudo TEST_DIR=/tmp/mnt/ SCRATCH_MNT=/tmp/scratch SCRATCH_DEV=/tmp/scratch ./check -tmpfs generic/556
FSTYP         -- tmpfs
PLATFORM      -- Linux/x86_64 archlinux 6.12.0-rc4-00148-g46d491956216 #347 SMP PREEMPT_DYNAMIC Mon Nov  4 18:01:10 -03 2024
MKFS_OPTIONS  -- /tmp/scratch
MOUNT_OPTIONS -- -o size=1G /tmp/scratch /tmp/scratch

generic/556 0s ...  0s
mv: failed to preserve ownership for '/home/user/codes/xfstests-dev/results//check.time': Operation not permitted
Ran: generic/556
Passed all 1 tests

 common/casefold   | 11 +++++++++++
 common/rc         |  3 +++
 tests/generic/556 | 23 +++++++++++++++++------
 3 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/common/casefold b/common/casefold
index d9126f4c..d0be8538 100644
--- a/common/casefold
+++ b/common/casefold
@@ -12,6 +12,9 @@ _has_casefold_kernel_support()
 	f2fs)
 		test -f '/sys/fs/f2fs/features/casefold'
 		;;
+	tmpfs)
+		test -f '/sys/fs/tmpfs/features/casefold'
+		;;
 	*)
 		# defaults to unsupported
 		false
@@ -52,6 +55,10 @@ _scratch_mkfs_casefold()
 	f2fs)
 		_scratch_mkfs -C utf8 $*
 		;;
+	tmpfs)
+		# there's no mkfs for tmpfs, so we just mount it
+		mount -t tmpfs -o casefold tmpfs $SCRATCH_MNT
+		;;
 	*)
 		_notrun "Don't know how to mkfs with casefold support on $FSTYP"
 		;;
@@ -67,6 +74,10 @@ _scratch_mkfs_casefold_strict()
 	f2fs)
 		_scratch_mkfs -C utf8:strict
 		;;
+	tmpfs)
+		# there's no mkfs for tmpfs, so we just mount it
+		mount -t tmpfs -o casefold,strict_encoding tmpfs $SCRATCH_MNT
+		;;
 	*)
 		_notrun "Don't know how to mkfs with casefold-strict support on $FSTYP"
 		;;
diff --git a/common/rc b/common/rc
index 2af26f23..2ee46e51 100644
--- a/common/rc
+++ b/common/rc
@@ -374,6 +374,9 @@ _scratch_unmount()
 	btrfs)
 		$UMOUNT_PROG $SCRATCH_MNT
 		;;
+	tmpfs)
+		$UMOUNT_PROG $SCRATCH_MNT
+		;;
 	*)
 		$UMOUNT_PROG $SCRATCH_DEV
 		;;
diff --git a/tests/generic/556 b/tests/generic/556
index 51d2d482..78cdf1bf 100755
--- a/tests/generic/556
+++ b/tests/generic/556
@@ -485,11 +485,20 @@ test_strict_mode_invalid_filename()
 
 _scratch_mkfs_casefold >>$seqres.full 2>&1
 
-_scratch_mount
-
-_check_dmesg_for \
-	"\(${sdev}\): Using encoding defined by superblock: utf8" || \
-	_fail "Could not mount with encoding: utf8"
+# casefold tmpfs was already mounted in the mkfs step
+if [ "$FSTYP" != "tmpfs" ]; then
+	_scratch_mount
+fi
+
+if [ "$FSTYP" == "tmpfs" ]; then
+	_check_dmesg_for \
+		"tmpfs: Using encoding" || \
+		_fail "Could not mount with encoding: utf8"
+else
+	_check_dmesg_for \
+		"\(${sdev}\): Using encoding defined by superblock: utf8" || \
+		_fail "Could not mount with encoding: utf8"
+fi
 
 test_casefold_flag_basic
 test_casefold_lookup
@@ -517,7 +526,9 @@ _check_scratch_fs
 
 # Test Strict Mode
 _scratch_mkfs_casefold_strict >>$seqres.full 2>&1
-_scratch_mount
+if [ "$FSTYP" != "tmpfs" ]; then
+	_scratch_mount
+fi
 
 test_strict_mode_invalid_filename
 
-- 
2.47.0

Re: [PATCH v2] common/casefold: Support for tmpfs casefold test

[Gabriel Krisman Bertazi]

André Almeida <andrealmeid@igalia.com> writes:


> @@ -67,6 +74,10 @@ _scratch_mkfs_casefold_strict()
>  	f2fs)
>  		_scratch_mkfs -C utf8:strict
>  		;;
> +	tmpfs)
> +		# there's no mkfs for tmpfs, so we just mount it
> +		mount -t tmpfs -o casefold,strict_encoding tmpfs $SCRATCH_MNT
> +		;;


This looks like a hack.  Why mount it here instead of _scratch_mount?
This should just be a nop for tmpfs.


>  	*)
>  		_notrun "Don't know how to mkfs with casefold-strict support on $FSTYP"
>  		;;
> diff --git a/common/rc b/common/rc
> index 2af26f23..2ee46e51 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -374,6 +374,9 @@ _scratch_unmount()
>  	btrfs)
>  		$UMOUNT_PROG $SCRATCH_MNT
>  		;;
> +	tmpfs)
> +		$UMOUNT_PROG $SCRATCH_MNT
> +		;;
>  	*)
>  		$UMOUNT_PROG $SCRATCH_DEV
>  		;;
> diff --git a/tests/generic/556 b/tests/generic/556
> index 51d2d482..78cdf1bf 100755
> --- a/tests/generic/556
> +++ b/tests/generic/556
> @@ -485,11 +485,20 @@ test_strict_mode_invalid_filename()
>  
>  _scratch_mkfs_casefold >>$seqres.full 2>&1
>  
> -_scratch_mount
> -
> -_check_dmesg_for \
> -	"\(${sdev}\): Using encoding defined by superblock: utf8" || \
> -	_fail "Could not mount with encoding: utf8"
> +# casefold tmpfs was already mounted in the mkfs step
> +if [ "$FSTYP" != "tmpfs" ]; then
> +	_scratch_mount
> +fi

 can you fix _scratch_mount to correctly mount casefolded tmpfs?  if
not, provide a new local handler that will either call _scratch_mount
directly for !tmpfs or do the correct mount for tmpfs.

> +
> +if [ "$FSTYP" == "tmpfs" ]; then
> +	_check_dmesg_for \
> +		"tmpfs: Using encoding" || \
> +		_fail "Could not mount with encoding: utf8"
> +else
> +	_check_dmesg_for \
> +		"\(${sdev}\): Using encoding defined by superblock: utf8" || \
> +		_fail "Could not mount with encoding: utf8"
> +fi

Ugh. the kernel message should have been the same. Much better
than special casing the test.

>  
>  test_casefold_flag_basic
>  test_casefold_lookup
> @@ -517,7 +526,9 @@ _check_scratch_fs
>  
>  # Test Strict Mode
>  _scratch_mkfs_casefold_strict >>$seqres.full 2>&1
> -_scratch_mount
> +if [ "$FSTYP" != "tmpfs" ]; then
> +	_scratch_mount
> +fi
>  
>  test_strict_mode_invalid_filename

-- 
Gabriel Krisman Bertazi

Re: [PATCH v2] common/casefold: Support for tmpfs casefold test

[André Almeida]

Em 04/11/2024 18:45, Gabriel Krisman Bertazi escreveu:
> André Almeida <andrealmeid@igalia.com> writes:
> 
> 
>> @@ -67,6 +74,10 @@ _scratch_mkfs_casefold_strict()
>>   	f2fs)
>>   		_scratch_mkfs -C utf8:strict
>>   		;;
>> +	tmpfs)
>> +		# there's no mkfs for tmpfs, so we just mount it
>> +		mount -t tmpfs -o casefold,strict_encoding tmpfs $SCRATCH_MNT
>> +		;;
> 
> 
> This looks like a hack.  Why mount it here instead of _scratch_mount?
> This should just be a nop for tmpfs.
> 
> 

_scratch_mount doesn't have any code related to casefold, should I 
create a _scratch_mount_casefold for tmpfs?

I added it here because is where the other filesystem do their casefold 
options setup.

>>   	*)
>>   		_notrun "Don't know how to mkfs with casefold-strict support on $FSTYP"
>>   		;;
>> diff --git a/common/rc b/common/rc
>> index 2af26f23..2ee46e51 100644
>> --- a/common/rc
>> +++ b/common/rc
>> @@ -374,6 +374,9 @@ _scratch_unmount()
>>   	btrfs)
>>   		$UMOUNT_PROG $SCRATCH_MNT
>>   		;;
>> +	tmpfs)
>> +		$UMOUNT_PROG $SCRATCH_MNT
>> +		;;
>>   	*)
>>   		$UMOUNT_PROG $SCRATCH_DEV
>>   		;;
>> diff --git a/tests/generic/556 b/tests/generic/556
>> index 51d2d482..78cdf1bf 100755
>> --- a/tests/generic/556
>> +++ b/tests/generic/556
>> @@ -485,11 +485,20 @@ test_strict_mode_invalid_filename()
>>   
>>   _scratch_mkfs_casefold >>$seqres.full 2>&1
>>   
>> -_scratch_mount
>> -
>> -_check_dmesg_for \
>> -	"\(${sdev}\): Using encoding defined by superblock: utf8" || \
>> -	_fail "Could not mount with encoding: utf8"
>> +# casefold tmpfs was already mounted in the mkfs step
>> +if [ "$FSTYP" != "tmpfs" ]; then
>> +	_scratch_mount
>> +fi
> 
>   can you fix _scratch_mount to correctly mount casefolded tmpfs?  if
> not, provide a new local handler that will either call _scratch_mount
> directly for !tmpfs or do the correct mount for tmpfs.
> 
>> +
>> +if [ "$FSTYP" == "tmpfs" ]; then
>> +	_check_dmesg_for \
>> +		"tmpfs: Using encoding" || \
>> +		_fail "Could not mount with encoding: utf8"
>> +else
>> +	_check_dmesg_for \
>> +		"\(${sdev}\): Using encoding defined by superblock: utf8" || \
>> +		_fail "Could not mount with encoding: utf8"
>> +fi
> 
> Ugh. the kernel message should have been the same. Much better
> than special casing the test.
> 

Well, to be accurate the encoding isn't defined by the superblock in the 
case of tmpfs, so the message would be imprecise.

But I think the if wasn't needed after all, the _check_dmesg_for grep 
should be able to do fine like this:

"\(${sdev}\): Using encoding" || \
	_fail "Could not mount with encoding: utf8"


>>   
>>   test_casefold_flag_basic
>>   test_casefold_lookup
>> @@ -517,7 +526,9 @@ _check_scratch_fs
>>   
>>   # Test Strict Mode
>>   _scratch_mkfs_casefold_strict >>$seqres.full 2>&1
>> -_scratch_mount
>> +if [ "$FSTYP" != "tmpfs" ]; then
>> +	_scratch_mount
>> +fi
>>   
>>   test_strict_mode_invalid_filename
> 

Re: [PATCH v2] common/casefold: Support for tmpfs casefold test

[Gabriel Krisman Bertazi]

André Almeida <andrealmeid@igalia.com> writes:

> Em 04/11/2024 18:45, Gabriel Krisman Bertazi escreveu:
>> André Almeida <andrealmeid@igalia.com> writes:
>> 
>>> @@ -67,6 +74,10 @@ _scratch_mkfs_casefold_strict()
>>>   	f2fs)
>>>   		_scratch_mkfs -C utf8:strict
>>>   		;;
>>> +	tmpfs)
>>> +		# there's no mkfs for tmpfs, so we just mount it
>>> +		mount -t tmpfs -o casefold,strict_encoding tmpfs $SCRATCH_MNT
>>> +		;;
>> This looks like a hack.  Why mount it here instead of _scratch_mount?
>> This should just be a nop for tmpfs.
>> 
>
> _scratch_mount doesn't have any code related to casefold, should I
> create a _scratch_mount_casefold for tmpfs?
>

Yep. That's what I'm suggesting at:

>>   can you fix _scratch_mount to correctly mount casefolded tmpfs?  if
>> not, provide a new local handler that will either call _scratch_mount
>> directly for !tmpfs or do the correct mount for tmpfs.

>>> +
>>> +if [ "$FSTYP" == "tmpfs" ]; then
>>> +	_check_dmesg_for \
>>> +		"tmpfs: Using encoding" || \
>>> +		_fail "Could not mount with encoding: utf8"
>>> +else
>>> +	_check_dmesg_for \
>>> +		"\(${sdev}\): Using encoding defined by superblock: utf8" || \
>>> +		_fail "Could not mount with encoding: utf8"
>>> +fi
>> Ugh. the kernel message should have been the same. Much better
>> than special casing the test.
>> 
>
> Well, to be accurate the encoding isn't defined by the superblock in the
> case of tmpfs, so the message would be imprecise.
>
> But I think the if wasn't needed after all, the _check_dmesg_for grep
> should be able to do fine like this:
>
> "\(${sdev}\): Using encoding" || \
> 	_fail "Could not mount with encoding: utf8"

that's better :)


-- 
Gabriel Krisman Bertazi