[PATCH] generic/683: _require_chmod

[PATCH] generic/683: _require_chmod

[Eric Sandeen]

generic/683 uses chmod but does not _require_chmod, so filesystems
like exfat fail.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

diff --git a/tests/generic/683 b/tests/generic/683
index 883905da..226d2758 100755
--- a/tests/generic/683
+++ b/tests/generic/683
@@ -24,6 +24,7 @@ _cleanup()
 # Modify as appropriate.
 _require_user
 _require_test
+_require_chmod
 verb=falloc
 _require_xfs_io_command $verb
 _require_congruent_file_oplen $TEST_DIR 65536

Re: [PATCH] generic/683: _require_chmod

[Zorro Lang]

On Wed, Aug 19, 2026 at 03:32:01PM -0500, Eric Sandeen wrote:
> generic/683 uses chmod but does not _require_chmod, so filesystems
> like exfat fail.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---

Hi Eric,

So great to see you ! This patch makes sense to me. Actually, g/683 belongs
to the same group/patchset of SUID test cases as g/684 through g/688, and all
of them use chmod. If we need to add _require_chmod to g/683, I would suggest
adding it to all of them.

Thanks,
Zorro

> 
> diff --git a/tests/generic/683 b/tests/generic/683
> index 883905da..226d2758 100755
> --- a/tests/generic/683
> +++ b/tests/generic/683
> @@ -24,6 +24,7 @@ _cleanup()
>  # Modify as appropriate.
>  _require_user
>  _require_test
> +_require_chmod
>  verb=falloc
>  _require_xfs_io_command $verb
>  _require_congruent_file_oplen $TEST_DIR 65536
> 

Re: [PATCH] generic/683: _require_chmod

[Eric Sandeen]

On 8/20/26 9:37 AM, Zorro Lang wrote:
> On Wed, Aug 19, 2026 at 03:32:01PM -0500, Eric Sandeen wrote:
>> generic/683 uses chmod but does not _require_chmod, so filesystems
>> like exfat fail.
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
> 
> Hi Eric,
> 
> So great to see you ! This patch makes sense to me. Actually, g/683 belongs
> to the same group/patchset of SUID test cases as g/684 through g/688, and all
> of them use chmod. If we need to add _require_chmod to g/683, I would suggest
> adding it to all of them.

Ok, fair enough - I should have looked more closely, and when running exfat
all of those tests properly did a _notrun due to /other/ constraints, but
we probably should explicitly mark each requirement even if they are
stacked.

I wonder if some sort of checkpatch or linter for xfstests might be
helpful, it's pretty simple to grep a test for chmod and ensure that
it has _require_chmod for example.

... or I wonder if it would be runtime-prohibitive to just build that
into the check script; if the script contains "chmod," run _require_chmod
automatically... similar for all the other trivial _require tests.

But maybe that's trying to be too clever. :)

-Eric

> Thanks,
> Zorro
> 
>>
>> diff --git a/tests/generic/683 b/tests/generic/683
>> index 883905da..226d2758 100755
>> --- a/tests/generic/683
>> +++ b/tests/generic/683
>> @@ -24,6 +24,7 @@ _cleanup()
>>  # Modify as appropriate.
>>  _require_user
>>  _require_test
>> +_require_chmod
>>  verb=falloc
>>  _require_xfs_io_command $verb
>>  _require_congruent_file_oplen $TEST_DIR 65536
>>

Re: [PATCH] generic/683: _require_chmod

[Zorro Lang]

On Thu, Aug 20, 2026 at 09:57:56AM -0500, Eric Sandeen wrote:
> On 8/20/26 9:37 AM, Zorro Lang wrote:
> > On Wed, Aug 19, 2026 at 03:32:01PM -0500, Eric Sandeen wrote:
> >> generic/683 uses chmod but does not _require_chmod, so filesystems
> >> like exfat fail.
> >>
> >> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> >> ---
> > 
> > Hi Eric,
> > 
> > So great to see you ! This patch makes sense to me. Actually, g/683 belongs
> > to the same group/patchset of SUID test cases as g/684 through g/688, and all
> > of them use chmod. If we need to add _require_chmod to g/683, I would suggest
> > adding it to all of them.
> 
> Ok, fair enough - I should have looked more closely, and when running exfat
> all of those tests properly did a _notrun due to /other/ constraints, but
> we probably should explicitly mark each requirement even if they are
> stacked.

Maybe some of the other _require_ checks helped those tests to _notrun on exfat.

> 
> I wonder if some sort of checkpatch or linter for xfstests might be
> helpful, it's pretty simple to grep a test for chmod and ensure that
> it has _require_chmod for example.

Sure, I just tried this:

  #!/bin/bash

  TESTS_DIR="${1:-tests}"

  find "$TESTS_DIR" -type f -regex '.*/[0-9]+$' | sort | while read -r test_file; do
        if grep -qE '^[[:space:]]*_require_chmod\b' "$test_file"; then
                continue
        fi

        if grep -v '^[[:space:]]*#' "$test_file" | grep -qE '\b(chmod|\$CHMOD_PROG)\b'; then
                echo "[Miss _require_chmod] $test_file"
                grep -v '^[[:space:]]*#' "$test_file" | grep -nE '\b(chmod|\$CHMOD_PROG)\b' | sed 's/^/    Line /'
        fi
  done

Not sure if we should blindly add _require_chmod to every single case we found.

>
> ... or I wonder if it would be runtime-prohibitive to just build that
> into the check script; if the script contains "chmod," run _require_chmod
> automatically... similar for all the other trivial _require tests.
> 
> But maybe that's trying to be too clever. :)

xfstests contains thousands of test cases, so this approach would introduce
substantial redundant overhead and frustrate a lot of developers :)

Thanks,
Zorro

> 
> -Eric
> 
> > Thanks,
> > Zorro
> > 
> >>
> >> diff --git a/tests/generic/683 b/tests/generic/683
> >> index 883905da..226d2758 100755
> >> --- a/tests/generic/683
> >> +++ b/tests/generic/683
> >> @@ -24,6 +24,7 @@ _cleanup()
> >>  # Modify as appropriate.
> >>  _require_user
> >>  _require_test
> >> +_require_chmod
> >>  verb=falloc
> >>  _require_xfs_io_command $verb
> >>  _require_congruent_file_oplen $TEST_DIR 65536
> >>
> 
>