[xfstests,1/2] common/casefold: Add infrastructure to test filename casefold feature
diff mbox series

Message ID 20190506185941.10570-1-krisman@collabora.com
State New
Headers show
Series
  • [xfstests,1/2] common/casefold: Add infrastructure to test filename casefold feature
Related show

Commit Message

Gabriel Krisman Bertazi May 6, 2019, 6:59 p.m. UTC
Add a set of basic helper functions to simplify the testing of
casefolding capable filesystems.

Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>
---
 common/casefold | 74 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)
 create mode 100644 common/casefold

Comments

Theodore Y. Ts'o May 6, 2019, 8:03 p.m. UTC | #1
On Mon, May 06, 2019 at 02:59:40PM -0400, Gabriel Krisman Bertazi wrote:
> +_require_test_casefold_feature () {
> +    _has_casefold_feature $TEST_DEV || \
> +	_notrun "Feature casefold required for this test"
> +}
> +_require_scratch_casefold_feature () {
> +    _has_casefold_feature $SCRATCH_DEV || \
> +	_notrun "Feature casefold required for this test"
> +}

I've just pushed out a commit to ext4.git tree which will cause
/sys/fs/ext4/features/casefold will exist iff CONFIG_UNICODE is
present.  This will allow the test to check whether or not the kernel
version and configuration will support the casefold feature.

Could you add a check for this flag if the file system type is ext4?

A file system independent way of doing this would be to create a test
file system on the test file system, calling "chattr +F" on the
directory.  If it fails, then either the file system doesn't support
it or the chattr program is too old and doesn't support casefold.  If
the chattr +F succeeds, then the test should call lsattr -d on the
directory and make sure the request to set casefold flag was actually
honored; some file systems will simply fail to set flags that they
don't support, so we do need to do a SETFLAGS followed by a GETFLAGS
to be sure that it was supported.

Speaking of file system independent casefold, I believe that it will
be likely that the casefold feature will be supported by f2fs in the
fullness of time.  If that happens, how to test for the file system
feature will be different (since dumpe2fs is ext4-specific), but I
would expect "chattr +F" interface to be the same between ext4 and
f2fs.

This might mean that we should add casefold tests to either generic/
or shared/ instead of ext4/ --- I think it would be shared since at
least initially it would only be ext4 and f2fs, and I haven't seen any
indication than other file systems would be interested in adding
casefold support.  Or we can move the casefold tests later from ext4/
to shared/ once the f2fs support materializes.

Cheers,

						- Ted
Gabriel Krisman Bertazi May 6, 2019, 8:32 p.m. UTC | #2
"Theodore Ts'o" <tytso@mit.edu> writes:

> On Mon, May 06, 2019 at 02:59:40PM -0400, Gabriel Krisman Bertazi wrote:
>> +_require_test_casefold_feature () {
>> +    _has_casefold_feature $TEST_DEV || \
>> +	_notrun "Feature casefold required for this test"
>> +}
>> +_require_scratch_casefold_feature () {
>> +    _has_casefold_feature $SCRATCH_DEV || \
>> +	_notrun "Feature casefold required for this test"
>> +}
>
> I've just pushed out a commit to ext4.git tree which will cause
> /sys/fs/ext4/features/casefold will exist iff CONFIG_UNICODE is
> present.  This will allow the test to check whether or not the kernel
> version and configuration will support the casefold feature.
>
> Could you add a check for this flag if the file system type is ext4?

Hello Ted,

I will follow up with this change on a v2.

> A file system independent way of doing this would be to create a test
> file system on the test file system, calling "chattr +F" on the
> directory.  If it fails, then either the file system doesn't support
> it or the chattr program is too old and doesn't support casefold.  If
> the chattr +F succeeds, then the test should call lsattr -d on the
> directory and make sure the request to set casefold flag was actually
> honored; some file systems will simply fail to set flags that they
> don't support, so we do need to do a SETFLAGS followed by a GETFLAGS
> to be sure that it was supported.

> Speaking of file system independent casefold, I believe that it will
> be likely that the casefold feature will be supported by f2fs in the
> fullness of time.  If that happens, how to test for the file system
> feature will be different (since dumpe2fs is ext4-specific), but I
> would expect "chattr +F" interface to be the same between ext4 and
> f2fs.

I planned to add the per-filesystem test inside common/casefold.  Not
sure how it would be done for f2fs, but i don't think we'd have a
unified interface other than SETFLAGS followed by GETFLAGS to test
this.  I think I could make this method the fallback.

>
> This might mean that we should add casefold tests to either generic/
> or shared/ instead of ext4/ --- I think it would be shared since at
> least initially it would only be ext4 and f2fs, and I haven't seen any
> indication than other file systems would be interested in adding
> casefold support.  Or we can move the casefold tests later from ext4/
> to shared/ once the f2fs support materializes.

Last thing I did before submitting this series was moving from generic/
to ext4/.  I plan to move it back into generic/ or shared/ once another
filesystem uses it.

I didn't have a chance to discuss with xfs folks yet, but I spoke to
Chris Mason and I plan to propose this feature for xfs and btrfs soon.

Patch
diff mbox series

diff --git a/common/casefold b/common/casefold
new file mode 100644
index 000000000000..c2436179265e
--- /dev/null
+++ b/common/casefold
@@ -0,0 +1,74 @@ 
+#-----------------------------------------------------------------------
+#
+# Common functions for testing filename casefold feature
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2018 Collabora, Ltd.  All Rights Reserved.
+#
+# Author: Gabriel Krisman Bertazi <krisman@collabora.com>
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+
+_has_casefold_feature() {
+    dumpe2fs -h "$1" 2>&1 | grep -Gq "Filesystem features.*casefold"
+}
+
+_require_test_casefold_feature () {
+    _has_casefold_feature $TEST_DEV || \
+	_notrun "Feature casefold required for this test"
+}
+_require_scratch_casefold_feature () {
+    _has_casefold_feature $SCRATCH_DEV || \
+	_notrun "Feature casefold required for this test"
+}
+_exclude_test_casefold_feature () {
+    _has_casefold_feature $TEST_DEV && \
+	_notrun "Feature casefold forbidden for this test"
+}
+_exclude_scratch_casefold_feature () {
+    _has_casefold_feature $SCRATCH_DEV && \
+	_notrun "Feature casefold forbidden for this test"
+}
+
+_casefold_check_exact_name () {
+    # To get the exact disk name, we need some method that does a
+    # getdents() on the parent directory, such that we don't get
+    # normalized/casefolded results.  'Find' works ok.
+    basedir=$1
+    exact_name=$2
+    find ${basedir} | grep -q ${exact_name}
+}
+
+_try_set_casefold_attr () {
+    chattr +F "${1}" &>/dev/null
+}
+
+_try_unset_casefold_attr () {
+    chattr -F "${1}" &>/dev/null
+}
+
+_set_casefold_attr () {
+    _try_set_casefold_attr "${1}" || \
+	_fail "Unable to set casefold attribute on ${1}"
+}
+
+_unset_casefold_attr () {
+    _try_unset_casefold_attr "${1}" || \
+	_fail "Unable to unset casefold attribute on ${1}"
+}
+
+_is_casefolded_dir () {
+    lsattr -ld "${1}" | grep -q "Casefold"
+}