[PATCH 0/1]: generic/453: Do NOT run for FSs restricting names

[PATCH 0/1]: generic/453: Do NOT run for FSs restricting names

[Pavel Reichl]

Hello,

this is a quick attempt to finish what was discussed in thread:
	generic/453: Exclude filenames that are not supported by exfat
	https://www.spinics.net/lists/fstests/msg16328.html

Based on Darrick's comments:

"""
TBH I think these tests (g/453 and g/454) are probably only useful for
filesystems that allow unrestricted byte streams for names.
"""

and

"""
Nowadays with wider testing of other filesystems (thanks, Red Hat!) we
should hide these behind _require_names_are_bytes or move them to
tests/xfs/.
"""


Pavel Reichl (1):
  generic/453: Do NOT run for FSs restricting names

 common/rc         | 11 +++++++++++
 tests/generic/453 |  1 +
 2 files changed, 12 insertions(+)

-- 
2.31.1

[PATCH 1/1] generic/453: Do NOT run for FSs restricting names

[Pavel Reichl]

Running generic/453 test is probably only useful for filesystems that
allow unrestricted byte streams for names.

Signed-off-by: Pavel Reichl <preichl@redhat.com>
Suggested-by: Darrick J. Wong <djwong@kernel.org>
---
 common/rc         | 11 +++++++++++
 tests/generic/453 |  1 +
 2 files changed, 12 insertions(+)

diff --git a/common/rc b/common/rc
index d4ad421e..7a352445 100644
--- a/common/rc
+++ b/common/rc
@@ -4599,6 +4599,17 @@ _require_od_endian_flag()
 		_notrun "od does not support endian flag"
 }
 
+_require_names_are_bytes() {
+        case "$FSTYP" in
+        ext2|ext3|ext4|xfs|btrfs)
+		# do nothing
+	        ;;
+	*)
+                _notrun "$FSTYP does now allow unrestricted byte streams for names"
+		;;
+        esac
+}
+
 init_rc
 
 ################################################################################
diff --git a/tests/generic/453 b/tests/generic/453
index 55ddcc01..a0fb802e 100755
--- a/tests/generic/453
+++ b/tests/generic/453
@@ -15,6 +15,7 @@ _begin_fstest auto quick dir
 # Import common functions.
 
 _require_scratch
+_require_names_are_bytes
 
 echo "Format and mount"
 _scratch_mkfs > $seqres.full 2>&1
-- 
2.31.1

Re: [PATCH 1/1] generic/453: Do NOT run for FSs restricting names

[Darrick J. Wong]

On Wed, Jul 07, 2021 at 12:20:07PM +0200, Pavel Reichl wrote:
> Running generic/453 test is probably only useful for filesystems that
> allow unrestricted byte streams for names.
> 
> Signed-off-by: Pavel Reichl <preichl@redhat.com>
> Suggested-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  common/rc         | 11 +++++++++++
>  tests/generic/453 |  1 +
>  2 files changed, 12 insertions(+)
> 
> diff --git a/common/rc b/common/rc
> index d4ad421e..7a352445 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -4599,6 +4599,17 @@ _require_od_endian_flag()
>  		_notrun "od does not support endian flag"
>  }
>  
> +_require_names_are_bytes() {

This helper needs a short description.

# Skip this test unless the filesystem treats names (directory entries,
# fs labels, and extended attribute names) as raw byte sequences.

> +        case "$FSTYP" in
> +        ext2|ext3|ext4|xfs|btrfs)

Does this need to _notrun ext4 filesystems that have casefolding
enabled?  (Or: should we let the ext4 developers figure that out?)

> +		# do nothing
> +	        ;;
> +	*)
> +                _notrun "$FSTYP does now allow unrestricted byte streams for names"
> +		;;
> +        esac
> +}
> +
>  init_rc
>  
>  ################################################################################
> diff --git a/tests/generic/453 b/tests/generic/453
> index 55ddcc01..a0fb802e 100755
> --- a/tests/generic/453
> +++ b/tests/generic/453
> @@ -15,6 +15,7 @@ _begin_fstest auto quick dir
>  # Import common functions.
>  
>  _require_scratch
> +_require_names_are_bytes

Do you need this for generic/454 as well?

--D

>  
>  echo "Format and mount"
>  _scratch_mkfs > $seqres.full 2>&1
> -- 
> 2.31.1
> 

Re: [PATCH 1/1] generic/453: Do NOT run for FSs restricting names

[Theodore Ts'o]

On Wed, Jul 07, 2021 at 12:20:07PM +0200, Pavel Reichl wrote:
> +_require_names_are_bytes() {
> +        case "$FSTYP" in
> +        ext2|ext3|ext4|xfs|btrfs)
> +		# do nothing
> +	        ;;
> +	*)
> +                _notrun "$FSTYP does now allow unrestricted byte streams for names"

"now" should be "not" above, I believe.

Other than that,

Reviewed-by: Theodore Ts'o <tytso@mit.edu>

						- Ted

Re: [PATCH 1/1] generic/453: Do NOT run for FSs restricting names

[Theodore Ts'o]

On Wed, Jul 07, 2021 at 07:51:58AM -0700, Darrick J. Wong wrote:
> # Skip this test unless the filesystem treats names (directory entries,
> # fs labels, and extended attribute names) as raw byte sequences.
> 
> > +        case "$FSTYP" in
> > +        ext2|ext3|ext4|xfs|btrfs)
> 
> Does this need to _notrun ext4 filesystems that have casefolding
> enabled?  (Or: should we let the ext4 developers figure that out?)

Casefolding has to be enabled on the file system level (mkfs.ext4 -O
casefold) but also on a per-directory level (by setting the casefold
flag).  Otherwise, unrestricted byte streams are allowed for file
names.  Since generic/453 doesn't set the casefold flag, this tests
passes even when the scratch mount options include casefolding.

The generic/556 test will test casefolding.

BTW, we should probably include f2fs as a file system which normally
allow unrestricted byte streams.  F2fs also supports casefolding, but
again, like ext4, it's only enabled when the per-directory casefold
flag is enabled.  So:

+        case "$FSTYP" in
+        ext2|ext3|ext4|f2fs|xfs|btrfs)

I've verified that generic/453 passes on f2fs today.

				- Ted

Re: [PATCH 1/1] generic/453: Do NOT run for FSs restricting names

[Pavel Reichl]

On 7/8/21 12:40 AM, Theodore Ts'o wrote:
> BTW, we should probably include f2fs as a file system which normally
> allow unrestricted byte streams.  F2fs also supports casefolding, but
> again, like ext4, it's only enabled when the per-directory casefold
> flag is enabled.  So:
>
> +        case "$FSTYP" in
> +        ext2|ext3|ext4|f2fs|xfs|btrfs)
>
> I've verified that generic/453 passes on f2fs today.
>
> 				- Ted
>
Hi Ted, thanks for the info. I'll update the patch to contain f2fs.