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

[Gabriel Krisman Bertazi <gabriel@krisman.be>]

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