| Message ID | 20241104210636.38934-1-andrealmeid@igalia.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] common/casefold: Support for tmpfs casefold test | expand |
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
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 >
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 :)
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
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(-)