generic: suggest fs specific fix only if the tested filesystem matches

Commit Message

Filipe Manana Feb. 6, 2025, 1:05 p.m. UTC

From: Filipe Manana <[e-mail]>

It's odd when a test fails on a filesystem and a specific fix is suggested
for another filesystem. Some generic tests are suggesting filesystem
specific fixes without checking if the running filesystem matches, so
update them.

Signed-off-by: Filipe Manana <[e-mail]>
---
 tests/generic/365 | 10 ++++++----
 tests/generic/366 |  2 +-
 tests/generic/367 |  2 +-
 tests/generic/623 |  2 +-
 tests/generic/631 |  2 +-
 tests/generic/646 |  2 +-
 tests/generic/649 |  2 +-
 tests/generic/695 |  2 +-
 tests/generic/700 |  4 ++--
 tests/generic/701 |  2 +-
 tests/generic/702 |  2 +-
 tests/generic/704 |  4 +++-
 tests/generic/707 |  4 ++--
 13 files changed, 22 insertions(+), 18 deletions(-)

Comments

Christoph Hellwig Feb. 6, 2025, 2:11 p.m. UTC | #1

On Thu, Feb 06, 2025 at 01:05:06PM +0000, [e-mail] wrote:
> +if [ "$FSTYP" = "xfs" ]; then
> +	_fixed_by_kernel_commit 68415b349f3f \
> +		"xfs: Fix the owner setting issue for rmap query in xfs fsmap"
> +	_fixed_by_kernel_commit ca6448aed4f1 \
> +		"xfs: Fix missing interval for missing_owner in xfs fsmap"
> +fi

How about you add a new helper instead of the boilerplate, something
like

_fixed_by_fs_kernel_commit xfs 68415b349f3f \
	"xfs: Fix the owner setting issue for rmap query in xfs fsmap"

?

Darrick J. Wong Feb. 6, 2025, 3:55 p.m. UTC | #2

On Thu, Feb 06, 2025 at 06:11:11AM -0800, Christoph Hellwig wrote:
> On Thu, Feb 06, 2025 at 01:05:06PM +0000, [e-mail] wrote:
> > +if [ "$FSTYP" = "xfs" ]; then
> > +	_fixed_by_kernel_commit 68415b349f3f \
> > +		"xfs: Fix the owner setting issue for rmap query in xfs fsmap"
> > +	_fixed_by_kernel_commit ca6448aed4f1 \
> > +		"xfs: Fix missing interval for missing_owner in xfs fsmap"
> > +fi
> 
> How about you add a new helper instead of the boilerplate, something
> like
> 
> _fixed_by_fs_kernel_commit xfs 68415b349f3f \
> 	"xfs: Fix the owner setting issue for rmap query in xfs fsmap"

While we're at it, we should move these _fixed_by* functions to a
separate file to declutter common/rc?

And maybe add a few more dumb wrappers like

_fixed_by_xfsprogs_commit()
{
	test "$FSTYP" = "xfs" && \
		__fixed_by_git_commit xfsprogs "$@"
}

to replace the opencoded versions?

--D

> ?
>

Qu Wenruo Feb. 6, 2025, 9:12 p.m. UTC | #3

在 2025/2/7 00:41, Christoph Hellwig 写道:
> On Thu, Feb 06, 2025 at 01:05:06PM +0000, [e-mail] wrote:
>> +if [ "$FSTYP" = "xfs" ]; then
>> +	_fixed_by_kernel_commit 68415b349f3f \
>> +		"xfs: Fix the owner setting issue for rmap query in xfs fsmap"
>> +	_fixed_by_kernel_commit ca6448aed4f1 \
>> +		"xfs: Fix missing interval for missing_owner in xfs fsmap"
>> +fi
>
> How about you add a new helper instead of the boilerplate, something
> like
>
> _fixed_by_fs_kernel_commit xfs 68415b349f3f \
> 	"xfs: Fix the owner setting issue for rmap query in xfs fsmap"
>
> ?
>
But what if the fix is generic? E.g. in mm/VFS layer?

Should we choose some placeholder name like "generic" as a fs type instead?

Thanks,
Qu

Darrick J. Wong Feb. 6, 2025, 10:33 p.m. UTC | #4

On Fri, Feb 07, 2025 at 07:42:29AM +1030, Qu Wenruo wrote:
> 
> 
> 在 2025/2/7 00:41, Christoph Hellwig 写道:
> > On Thu, Feb 06, 2025 at 01:05:06PM +0000, [e-mail] wrote:
> > > +if [ "$FSTYP" = "xfs" ]; then
> > > +	_fixed_by_kernel_commit 68415b349f3f \
> > > +		"xfs: Fix the owner setting issue for rmap query in xfs fsmap"
> > > +	_fixed_by_kernel_commit ca6448aed4f1 \
> > > +		"xfs: Fix missing interval for missing_owner in xfs fsmap"
> > > +fi
> > 
> > How about you add a new helper instead of the boilerplate, something
> > like
> > 
> > _fixed_by_fs_kernel_commit xfs 68415b349f3f \
> > 	"xfs: Fix the owner setting issue for rmap query in xfs fsmap"
> > 
> > ?
> > 
> But what if the fix is generic? E.g. in mm/VFS layer?
> 
> Should we choose some placeholder name like "generic" as a fs type instead?

Keep using _fixed_by_kernel_commit then.

--D

> Thanks,
> Qu
>

Zorro Lang Feb. 10, 2025, 2:37 p.m. UTC | #5

On Thu, Feb 06, 2025 at 07:55:01AM -0800, Darrick J. Wong wrote:
> On Thu, Feb 06, 2025 at 06:11:11AM -0800, Christoph Hellwig wrote:
> > On Thu, Feb 06, 2025 at 01:05:06PM +0000, [e-mail] wrote:
> > > +if [ "$FSTYP" = "xfs" ]; then
> > > +	_fixed_by_kernel_commit 68415b349f3f \
> > > +		"xfs: Fix the owner setting issue for rmap query in xfs fsmap"
> > > +	_fixed_by_kernel_commit ca6448aed4f1 \
> > > +		"xfs: Fix missing interval for missing_owner in xfs fsmap"
> > > +fi
> > 
> > How about you add a new helper instead of the boilerplate, something
> > like
> > 
> > _fixed_by_fs_kernel_commit xfs 68415b349f3f \
> > 	"xfs: Fix the owner setting issue for rmap query in xfs fsmap"

This looks good to me ^^

> 
> While we're at it, we should move these _fixed_by* functions to a
> separate file to declutter common/rc?
> 
> And maybe add a few more dumb wrappers like
> 
> _fixed_by_xfsprogs_commit()
> {
> 	test "$FSTYP" = "xfs" && \
> 		__fixed_by_git_commit xfsprogs "$@"

Yes, this makes sense to me.

Hi Filipe, I can review and merge this patch at first, if you'd like to
do above changes in another patch. Or I'll ignore this one, and wait your
next patchset. What's your plan?

Thanks,
Zorro

> }
> 
> to replace the opencoded versions?
> 
> --D
> 
> > ?
> > 
>

Filipe Manana Feb. 11, 2025, 10:55 a.m. UTC | #6

On Mon, Feb 10, 2025 at 2:37 PM Zorro Lang <[e-mail]> wrote:
>
> On Thu, Feb 06, 2025 at 07:55:01AM -0800, Darrick J. Wong wrote:
> > On Thu, Feb 06, 2025 at 06:11:11AM -0800, Christoph Hellwig wrote:
> > > On Thu, Feb 06, 2025 at 01:05:06PM +0000, [e-mail] wrote:
> > > > +if [ "$FSTYP" = "xfs" ]; then
> > > > + _fixed_by_kernel_commit 68415b349f3f \
> > > > +         "xfs: Fix the owner setting issue for rmap query in xfs fsmap"
> > > > + _fixed_by_kernel_commit ca6448aed4f1 \
> > > > +         "xfs: Fix missing interval for missing_owner in xfs fsmap"
> > > > +fi
> > >
> > > How about you add a new helper instead of the boilerplate, something
> > > like
> > >
> > > _fixed_by_fs_kernel_commit xfs 68415b349f3f \
> > >     "xfs: Fix the owner setting issue for rmap query in xfs fsmap"
>
> This looks good to me ^^
>
> >
> > While we're at it, we should move these _fixed_by* functions to a
> > separate file to declutter common/rc?
> >
> > And maybe add a few more dumb wrappers like
> >
> > _fixed_by_xfsprogs_commit()
> > {
> >       test "$FSTYP" = "xfs" && \
> >               __fixed_by_git_commit xfsprogs "$@"
>
> Yes, this makes sense to me.
>
> Hi Filipe, I can review and merge this patch at first, if you'd like to
> do above changes in another patch. Or I'll ignore this one, and wait your
> next patchset. What's your plan?

You may review and merge if it's ok for you.

As for the refactoring, it will take a while, I would likely have to
go over hundreds of generic tests, which I can't afford right now.

Thanks.

>
> Thanks,
> Zorro
>
> > }
> >
> > to replace the opencoded versions?
> >
> > --D
> >
> > > ?
> > >
> >
>

Zorro Lang Feb. 12, 2025, 1:53 a.m. UTC | #7

On Thu, Feb 06, 2025 at 01:05:06PM +0000, [e-mail] wrote:
> From: Filipe Manana <[e-mail]>
> 
> It's odd when a test fails on a filesystem and a specific fix is suggested
> for another filesystem. Some generic tests are suggesting filesystem
> specific fixes without checking if the running filesystem matches, so
> update them.
> 
> Signed-off-by: Filipe Manana <[e-mail]>
> ---
>  tests/generic/365 | 10 ++++++----
>  tests/generic/366 |  2 +-
>  tests/generic/367 |  2 +-
>  tests/generic/623 |  2 +-
>  tests/generic/631 |  2 +-
>  tests/generic/646 |  2 +-
>  tests/generic/649 |  2 +-
>  tests/generic/695 |  2 +-
>  tests/generic/700 |  4 ++--
>  tests/generic/701 |  2 +-
>  tests/generic/702 |  2 +-
>  tests/generic/704 |  4 +++-
>  tests/generic/707 |  4 ++--
>  13 files changed, 22 insertions(+), 18 deletions(-)
> 
> diff --git a/tests/generic/365 b/tests/generic/365
> index 1f6a618a..1bca848a 100755
> --- a/tests/generic/365
> +++ b/tests/generic/365
> @@ -9,10 +9,12 @@
>  . ./common/preamble
>  _begin_fstest auto rmap fsmap
>  
> -_fixed_by_kernel_commit 68415b349f3f \
> -	"xfs: Fix the owner setting issue for rmap query in xfs fsmap"
> -_fixed_by_kernel_commit ca6448aed4f1 \
> -	"xfs: Fix missing interval for missing_owner in xfs fsmap"
> +if [ "$FSTYP" = "xfs" ]; then
> +	_fixed_by_kernel_commit 68415b349f3f \
> +		"xfs: Fix the owner setting issue for rmap query in xfs fsmap"
> +	_fixed_by_kernel_commit ca6448aed4f1 \
> +		"xfs: Fix missing interval for missing_owner in xfs fsmap"
> +fi
>  
>  . ./common/filter
>  
> diff --git a/tests/generic/366 b/tests/generic/366
> index b322bcca..b2c2e607 100755
> --- a/tests/generic/366
> +++ b/tests/generic/366
> @@ -23,7 +23,7 @@ _require_scratch
>  _require_odirect 512	# see fio job1 config below
>  _require_aio
>  
> -_fixed_by_kernel_commit xxxxxxxxxxxx \
> +[ "$FSTYP" = "btrfs" ] && _fixed_by_kernel_commit xxxxxxxxxxxx \
>  	"btrfs: avoid deadlock when reading a partial uptodate folio"
>  
>  iterations=$((32 * LOAD_FACTOR))
> diff --git a/tests/generic/367 b/tests/generic/367
> index 7cf90695..ed371a02 100755
> --- a/tests/generic/367
> +++ b/tests/generic/367
> @@ -17,7 +17,7 @@
>  
>  _begin_fstest ioctl quick
>  
> -_fixed_by_kernel_commit 2a492ff66673 \
> +[ "$FSTYP" = "xfs" ] && _fixed_by_kernel_commit 2a492ff66673 \
>  	"xfs: Check for delayed allocations before setting extsize"
>  
>  _require_scratch_extsize
> diff --git a/tests/generic/623 b/tests/generic/623
> index 6487ccb8..9f41b5cc 100755
> --- a/tests/generic/623
> +++ b/tests/generic/623
> @@ -11,7 +11,7 @@ _begin_fstest auto quick shutdown
>  
>  . ./common/filter
>  
> -_fixed_by_kernel_commit e4826691cc7e \
> +[ "$FSTYP" = "xfs" ] && _fixed_by_kernel_commit e4826691cc7e \
>  	"xfs: restore shutdown check in mapped write fault path"
>  
>  _require_scratch_nocheck
> diff --git a/tests/generic/631 b/tests/generic/631
> index 8e2cf9c6..c38ab771 100755
> --- a/tests/generic/631
> +++ b/tests/generic/631
> @@ -41,7 +41,7 @@ _require_attrs trusted
>  _exclude_fs overlay
>  _require_extra_fs overlay
>  
> -_fixed_by_kernel_commit 6da1b4b1ab36 \
> +[ "$FSTYP" = "xfs" ] && _fixed_by_kernel_commit 6da1b4b1ab36 \
>  	"xfs: fix an ABBA deadlock in xfs_rename"
>  
>  _scratch_mkfs >> $seqres.full
> diff --git a/tests/generic/646 b/tests/generic/646
> index dc73aeb3..b3b0ab0a 100755
> --- a/tests/generic/646
> +++ b/tests/generic/646
> @@ -14,7 +14,7 @@
>  . ./common/preamble
>  _begin_fstest auto quick recoveryloop shutdown
>  
> -_fixed_by_kernel_commit 50d25484bebe \
> +[ "$FSTYP" = "xfs" ] && _fixed_by_kernel_commit 50d25484bebe \
>  	"xfs: sync lazy sb accounting on quiesce of read-only mounts"
>  
>  _require_scratch
> diff --git a/tests/generic/649 b/tests/generic/649
> index a33b13ea..58ef96a8 100755
> --- a/tests/generic/649
> +++ b/tests/generic/649
> @@ -31,7 +31,7 @@ _cleanup()
>  
>  
>  # Modify as appropriate.
> -_fixed_by_kernel_commit 72a048c1056a \
> +[ "$FSTYP" = "xfs" ] && _fixed_by_kernel_commit 72a048c1056a \
>  	"xfs: only set IOMAP_F_SHARED when providing a srcmap to a write"
>  
>  _require_cp_reflink
> diff --git a/tests/generic/695 b/tests/generic/695
> index df81fdb7..694f4245 100755
> --- a/tests/generic/695
> +++ b/tests/generic/695
> @@ -25,7 +25,7 @@ _cleanup()
>  . ./common/dmflakey
>  . ./common/punch
>  
> -_fixed_by_kernel_commit e6e3dec6c3c288 \
> +[ "$FSTYP" = "btrfs" ] && _fixed_by_kernel_commit e6e3dec6c3c288 \
>          "btrfs: update generation of hole file extent item when merging holes"
>  _require_scratch
>  _require_dm_target flakey
> diff --git a/tests/generic/700 b/tests/generic/700
> index 052cfbd6..7f84df9d 100755
> --- a/tests/generic/700
> +++ b/tests/generic/700
> @@ -19,8 +19,8 @@ _require_scratch
>  _require_attrs
>  _require_renameat2 whiteout
>  
> -_fixed_by_kernel_commit 70b589a37e1a \
> -	xfs: add selinux labels to whiteout inodes
> +[ "$FSTYP" = "xfs" ] && _fixed_by_kernel_commit 70b589a37e1a \
> +	"xfs: add selinux labels to whiteout inodes"
>  
>  get_selinux_label()
>  {
> diff --git a/tests/generic/701 b/tests/generic/701
> index 527bba34..806cc65d 100755
> --- a/tests/generic/701
> +++ b/tests/generic/701
> @@ -22,7 +22,7 @@ _cleanup()
>  	rm -r -f $tmp.* $junk_dir
>  }
>  
> -_fixed_by_kernel_commit 92fba084b79e \
> +[ "$FSTYP" = "exfat" ] && _fixed_by_kernel_commit 92fba084b79e \
>  	"exfat: fix i_blocks for files truncated over 4 GiB"
>  
>  _require_test
> diff --git a/tests/generic/702 b/tests/generic/702
> index a506e07d..ae47eb27 100755
> --- a/tests/generic/702
> +++ b/tests/generic/702
> @@ -14,7 +14,7 @@ _begin_fstest auto quick clone fiemap
>  . ./common/filter
>  . ./common/reflink
>  
> -_fixed_by_kernel_commit ac3c0d36a2a2f7 \
> +[ "$FSTYP" = "btrfs" ] && _fixed_by_kernel_commit ac3c0d36a2a2f7 \
>  	"btrfs: make fiemap more efficient and accurate reporting extent sharedness"
>  
>  _require_scratch_reflink
> diff --git a/tests/generic/704 b/tests/generic/704
> index f452f9e9..f2360c42 100755
> --- a/tests/generic/704
> +++ b/tests/generic/704
> @@ -21,7 +21,9 @@ _cleanup()
>  # Import common functions.
>  . ./common/scsi_debug
>  
> -_fixed_by_kernel_commit 7c71ee78031c "xfs: allow logical-sector sized O_DIRECT"
> +[ "$FSTYP" = "xfs" ] && _fixed_by_kernel_commit 7c71ee78031c \
> +	"xfs: allow logical-sector sized O_DIRECT"
> +
>  _require_scsi_debug
>  # If TEST_DEV is block device, make sure current fs is a localfs which can be
>  # written on scsi_debug device
> diff --git a/tests/generic/707 b/tests/generic/707
> index 3d8fac4b..23864038 100755
> --- a/tests/generic/707
> +++ b/tests/generic/707
> @@ -13,9 +13,9 @@ _begin_fstest auto
>  
>  _require_scratch
>  
> -_fixed_by_kernel_commit f950fd052913 \
> +[ "$FSTYP" = "udf" ] && _fixed_by_kernel_commit f950fd052913 \
>  	"udf: Protect rename against modification of moved directory"
> -_fixed_by_kernel_commit 0813299c586b \
> +[ "$FSTYP" = "ext4" ] && _fixed_by_kernel_commit 0813299c586b \

I'm wondering if it's a "ext4 only" bug, or it might can be [[ "$FSTYP" =~ ext[0-9]+ ]] ?
Others looks good to me.

Thanks,
Zorro

>  	"ext4: Fix possible corruption when moving a directory"
>  
>  _scratch_mkfs >>$seqres.full 2>&1
> -- 
> 2.45.2
> 
>

Filipe Manana Feb. 12, 2025, 12:38 p.m. UTC | #8

On Wed, Feb 12, 2025 at 1:53 AM Zorro Lang <[e-mail]> wrote:
>
> On Thu, Feb 06, 2025 at 01:05:06PM +0000, [e-mail] wrote:
> > From: Filipe Manana <[e-mail]>
> >
> > It's odd when a test fails on a filesystem and a specific fix is suggested
> > for another filesystem. Some generic tests are suggesting filesystem
> > specific fixes without checking if the running filesystem matches, so
> > update them.
> >
> > Signed-off-by: Filipe Manana <[e-mail]>
> > ---
> >  tests/generic/365 | 10 ++++++----
> >  tests/generic/366 |  2 +-
> >  tests/generic/367 |  2 +-
> >  tests/generic/623 |  2 +-
> >  tests/generic/631 |  2 +-
> >  tests/generic/646 |  2 +-
> >  tests/generic/649 |  2 +-
> >  tests/generic/695 |  2 +-
> >  tests/generic/700 |  4 ++--
> >  tests/generic/701 |  2 +-
> >  tests/generic/702 |  2 +-
> >  tests/generic/704 |  4 +++-
> >  tests/generic/707 |  4 ++--
> >  13 files changed, 22 insertions(+), 18 deletions(-)
> >
> > diff --git a/tests/generic/365 b/tests/generic/365
> > index 1f6a618a..1bca848a 100755
> > --- a/tests/generic/365
> > +++ b/tests/generic/365
> > @@ -9,10 +9,12 @@
> >  . ./common/preamble
> >  _begin_fstest auto rmap fsmap
> >
> > -_fixed_by_kernel_commit 68415b349f3f \
> > -     "xfs: Fix the owner setting issue for rmap query in xfs fsmap"
> > -_fixed_by_kernel_commit ca6448aed4f1 \
> > -     "xfs: Fix missing interval for missing_owner in xfs fsmap"
> > +if [ "$FSTYP" = "xfs" ]; then
> > +     _fixed_by_kernel_commit 68415b349f3f \
> > +             "xfs: Fix the owner setting issue for rmap query in xfs fsmap"
> > +     _fixed_by_kernel_commit ca6448aed4f1 \
> > +             "xfs: Fix missing interval for missing_owner in xfs fsmap"
> > +fi
> >
> >  . ./common/filter
> >
> > diff --git a/tests/generic/366 b/tests/generic/366
> > index b322bcca..b2c2e607 100755
> > --- a/tests/generic/366
> > +++ b/tests/generic/366
> > @@ -23,7 +23,7 @@ _require_scratch
> >  _require_odirect 512 # see fio job1 config below
> >  _require_aio
> >
> > -_fixed_by_kernel_commit xxxxxxxxxxxx \
> > +[ "$FSTYP" = "btrfs" ] && _fixed_by_kernel_commit xxxxxxxxxxxx \
> >       "btrfs: avoid deadlock when reading a partial uptodate folio"
> >
> >  iterations=$((32 * LOAD_FACTOR))
> > diff --git a/tests/generic/367 b/tests/generic/367
> > index 7cf90695..ed371a02 100755
> > --- a/tests/generic/367
> > +++ b/tests/generic/367
> > @@ -17,7 +17,7 @@
> >
> >  _begin_fstest ioctl quick
> >
> > -_fixed_by_kernel_commit 2a492ff66673 \
> > +[ "$FSTYP" = "xfs" ] && _fixed_by_kernel_commit 2a492ff66673 \
> >       "xfs: Check for delayed allocations before setting extsize"
> >
> >  _require_scratch_extsize
> > diff --git a/tests/generic/623 b/tests/generic/623
> > index 6487ccb8..9f41b5cc 100755
> > --- a/tests/generic/623
> > +++ b/tests/generic/623
> > @@ -11,7 +11,7 @@ _begin_fstest auto quick shutdown
> >
> >  . ./common/filter
> >
> > -_fixed_by_kernel_commit e4826691cc7e \
> > +[ "$FSTYP" = "xfs" ] && _fixed_by_kernel_commit e4826691cc7e \
> >       "xfs: restore shutdown check in mapped write fault path"
> >
> >  _require_scratch_nocheck
> > diff --git a/tests/generic/631 b/tests/generic/631
> > index 8e2cf9c6..c38ab771 100755
> > --- a/tests/generic/631
> > +++ b/tests/generic/631
> > @@ -41,7 +41,7 @@ _require_attrs trusted
> >  _exclude_fs overlay
> >  _require_extra_fs overlay
> >
> > -_fixed_by_kernel_commit 6da1b4b1ab36 \
> > +[ "$FSTYP" = "xfs" ] && _fixed_by_kernel_commit 6da1b4b1ab36 \
> >       "xfs: fix an ABBA deadlock in xfs_rename"
> >
> >  _scratch_mkfs >> $seqres.full
> > diff --git a/tests/generic/646 b/tests/generic/646
> > index dc73aeb3..b3b0ab0a 100755
> > --- a/tests/generic/646
> > +++ b/tests/generic/646
> > @@ -14,7 +14,7 @@
> >  . ./common/preamble
> >  _begin_fstest auto quick recoveryloop shutdown
> >
> > -_fixed_by_kernel_commit 50d25484bebe \
> > +[ "$FSTYP" = "xfs" ] && _fixed_by_kernel_commit 50d25484bebe \
> >       "xfs: sync lazy sb accounting on quiesce of read-only mounts"
> >
> >  _require_scratch
> > diff --git a/tests/generic/649 b/tests/generic/649
> > index a33b13ea..58ef96a8 100755
> > --- a/tests/generic/649
> > +++ b/tests/generic/649
> > @@ -31,7 +31,7 @@ _cleanup()
> >
> >
> >  # Modify as appropriate.
> > -_fixed_by_kernel_commit 72a048c1056a \
> > +[ "$FSTYP" = "xfs" ] && _fixed_by_kernel_commit 72a048c1056a \
> >       "xfs: only set IOMAP_F_SHARED when providing a srcmap to a write"
> >
> >  _require_cp_reflink
> > diff --git a/tests/generic/695 b/tests/generic/695
> > index df81fdb7..694f4245 100755
> > --- a/tests/generic/695
> > +++ b/tests/generic/695
> > @@ -25,7 +25,7 @@ _cleanup()
> >  . ./common/dmflakey
> >  . ./common/punch
> >
> > -_fixed_by_kernel_commit e6e3dec6c3c288 \
> > +[ "$FSTYP" = "btrfs" ] && _fixed_by_kernel_commit e6e3dec6c3c288 \
> >          "btrfs: update generation of hole file extent item when merging holes"
> >  _require_scratch
> >  _require_dm_target flakey
> > diff --git a/tests/generic/700 b/tests/generic/700
> > index 052cfbd6..7f84df9d 100755
> > --- a/tests/generic/700
> > +++ b/tests/generic/700
> > @@ -19,8 +19,8 @@ _require_scratch
> >  _require_attrs
> >  _require_renameat2 whiteout
> >
> > -_fixed_by_kernel_commit 70b589a37e1a \
> > -     xfs: add selinux labels to whiteout inodes
> > +[ "$FSTYP" = "xfs" ] && _fixed_by_kernel_commit 70b589a37e1a \
> > +     "xfs: add selinux labels to whiteout inodes"
> >
> >  get_selinux_label()
> >  {
> > diff --git a/tests/generic/701 b/tests/generic/701
> > index 527bba34..806cc65d 100755
> > --- a/tests/generic/701
> > +++ b/tests/generic/701
> > @@ -22,7 +22,7 @@ _cleanup()
> >       rm -r -f $tmp.* $junk_dir
> >  }
> >
> > -_fixed_by_kernel_commit 92fba084b79e \
> > +[ "$FSTYP" = "exfat" ] && _fixed_by_kernel_commit 92fba084b79e \
> >       "exfat: fix i_blocks for files truncated over 4 GiB"
> >
> >  _require_test
> > diff --git a/tests/generic/702 b/tests/generic/702
> > index a506e07d..ae47eb27 100755
> > --- a/tests/generic/702
> > +++ b/tests/generic/702
> > @@ -14,7 +14,7 @@ _begin_fstest auto quick clone fiemap
> >  . ./common/filter
> >  . ./common/reflink
> >
> > -_fixed_by_kernel_commit ac3c0d36a2a2f7 \
> > +[ "$FSTYP" = "btrfs" ] && _fixed_by_kernel_commit ac3c0d36a2a2f7 \
> >       "btrfs: make fiemap more efficient and accurate reporting extent sharedness"
> >
> >  _require_scratch_reflink
> > diff --git a/tests/generic/704 b/tests/generic/704
> > index f452f9e9..f2360c42 100755
> > --- a/tests/generic/704
> > +++ b/tests/generic/704
> > @@ -21,7 +21,9 @@ _cleanup()
> >  # Import common functions.
> >  . ./common/scsi_debug
> >
> > -_fixed_by_kernel_commit 7c71ee78031c "xfs: allow logical-sector sized O_DIRECT"
> > +[ "$FSTYP" = "xfs" ] && _fixed_by_kernel_commit 7c71ee78031c \
> > +     "xfs: allow logical-sector sized O_DIRECT"
> > +
> >  _require_scsi_debug
> >  # If TEST_DEV is block device, make sure current fs is a localfs which can be
> >  # written on scsi_debug device
> > diff --git a/tests/generic/707 b/tests/generic/707
> > index 3d8fac4b..23864038 100755
> > --- a/tests/generic/707
> > +++ b/tests/generic/707
> > @@ -13,9 +13,9 @@ _begin_fstest auto
> >
> >  _require_scratch
> >
> > -_fixed_by_kernel_commit f950fd052913 \
> > +[ "$FSTYP" = "udf" ] && _fixed_by_kernel_commit f950fd052913 \
> >       "udf: Protect rename against modification of moved directory"
> > -_fixed_by_kernel_commit 0813299c586b \
> > +[ "$FSTYP" = "ext4" ] && _fixed_by_kernel_commit 0813299c586b \
>
> I'm wondering if it's a "ext4 only" bug, or it might can be [[ "$FSTYP" =~ ext[0-9]+ ]] ?

Not sure either, but other generic test cases do that, so it's
probably best to do it like that.
Do you want a new patch version with that change or can you change it yourself?

Thanks.

> Others looks good to me.
>
> Thanks,
> Zorro
>
> >       "ext4: Fix possible corruption when moving a directory"
> >
> >  _scratch_mkfs >>$seqres.full 2>&1
> > --
> > 2.45.2
> >
> >
>

Zorro Lang Feb. 12, 2025, 2:12 p.m. UTC | #9

On Wed, Feb 12, 2025 at 12:38:27PM +0000, Filipe Manana wrote:
> On Wed, Feb 12, 2025 at 1:53 AM Zorro Lang <[e-mail]> wrote:
> >
> > On Thu, Feb 06, 2025 at 01:05:06PM +0000, [e-mail] wrote:
> > > From: Filipe Manana <[e-mail]>
> > >
> > > It's odd when a test fails on a filesystem and a specific fix is suggested
> > > for another filesystem. Some generic tests are suggesting filesystem
> > > specific fixes without checking if the running filesystem matches, so
> > > update them.
> > >
> > > Signed-off-by: Filipe Manana <[e-mail]>
> > > ---
> > >  tests/generic/365 | 10 ++++++----
> > >  tests/generic/366 |  2 +-
> > >  tests/generic/367 |  2 +-
> > >  tests/generic/623 |  2 +-
> > >  tests/generic/631 |  2 +-
> > >  tests/generic/646 |  2 +-
> > >  tests/generic/649 |  2 +-
> > >  tests/generic/695 |  2 +-
> > >  tests/generic/700 |  4 ++--
> > >  tests/generic/701 |  2 +-
> > >  tests/generic/702 |  2 +-
> > >  tests/generic/704 |  4 +++-
> > >  tests/generic/707 |  4 ++--
> > >  13 files changed, 22 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/tests/generic/365 b/tests/generic/365
> > > index 1f6a618a..1bca848a 100755
> > > --- a/tests/generic/365
> > > +++ b/tests/generic/365
> > > @@ -9,10 +9,12 @@
> > >  . ./common/preamble
> > >  _begin_fstest auto rmap fsmap
> > >
> > > -_fixed_by_kernel_commit 68415b349f3f \
> > > -     "xfs: Fix the owner setting issue for rmap query in xfs fsmap"
> > > -_fixed_by_kernel_commit ca6448aed4f1 \
> > > -     "xfs: Fix missing interval for missing_owner in xfs fsmap"
> > > +if [ "$FSTYP" = "xfs" ]; then
> > > +     _fixed_by_kernel_commit 68415b349f3f \
> > > +             "xfs: Fix the owner setting issue for rmap query in xfs fsmap"
> > > +     _fixed_by_kernel_commit ca6448aed4f1 \
> > > +             "xfs: Fix missing interval for missing_owner in xfs fsmap"
> > > +fi
> > >
> > >  . ./common/filter
> > >
> > > diff --git a/tests/generic/366 b/tests/generic/366
> > > index b322bcca..b2c2e607 100755
> > > --- a/tests/generic/366
> > > +++ b/tests/generic/366
> > > @@ -23,7 +23,7 @@ _require_scratch
> > >  _require_odirect 512 # see fio job1 config below
> > >  _require_aio
> > >
> > > -_fixed_by_kernel_commit xxxxxxxxxxxx \
> > > +[ "$FSTYP" = "btrfs" ] && _fixed_by_kernel_commit xxxxxxxxxxxx \
> > >       "btrfs: avoid deadlock when reading a partial uptodate folio"
> > >
> > >  iterations=$((32 * LOAD_FACTOR))
> > > diff --git a/tests/generic/367 b/tests/generic/367
> > > index 7cf90695..ed371a02 100755
> > > --- a/tests/generic/367
> > > +++ b/tests/generic/367
> > > @@ -17,7 +17,7 @@
> > >
> > >  _begin_fstest ioctl quick
> > >
> > > -_fixed_by_kernel_commit 2a492ff66673 \
> > > +[ "$FSTYP" = "xfs" ] && _fixed_by_kernel_commit 2a492ff66673 \
> > >       "xfs: Check for delayed allocations before setting extsize"
> > >
> > >  _require_scratch_extsize
> > > diff --git a/tests/generic/623 b/tests/generic/623
> > > index 6487ccb8..9f41b5cc 100755
> > > --- a/tests/generic/623
> > > +++ b/tests/generic/623
> > > @@ -11,7 +11,7 @@ _begin_fstest auto quick shutdown
> > >
> > >  . ./common/filter
> > >
> > > -_fixed_by_kernel_commit e4826691cc7e \
> > > +[ "$FSTYP" = "xfs" ] && _fixed_by_kernel_commit e4826691cc7e \
> > >       "xfs: restore shutdown check in mapped write fault path"
> > >
> > >  _require_scratch_nocheck
> > > diff --git a/tests/generic/631 b/tests/generic/631
> > > index 8e2cf9c6..c38ab771 100755
> > > --- a/tests/generic/631
> > > +++ b/tests/generic/631
> > > @@ -41,7 +41,7 @@ _require_attrs trusted
> > >  _exclude_fs overlay
> > >  _require_extra_fs overlay
> > >
> > > -_fixed_by_kernel_commit 6da1b4b1ab36 \
> > > +[ "$FSTYP" = "xfs" ] && _fixed_by_kernel_commit 6da1b4b1ab36 \
> > >       "xfs: fix an ABBA deadlock in xfs_rename"
> > >
> > >  _scratch_mkfs >> $seqres.full
> > > diff --git a/tests/generic/646 b/tests/generic/646
> > > index dc73aeb3..b3b0ab0a 100755
> > > --- a/tests/generic/646
> > > +++ b/tests/generic/646
> > > @@ -14,7 +14,7 @@
> > >  . ./common/preamble
> > >  _begin_fstest auto quick recoveryloop shutdown
> > >
> > > -_fixed_by_kernel_commit 50d25484bebe \
> > > +[ "$FSTYP" = "xfs" ] && _fixed_by_kernel_commit 50d25484bebe \
> > >       "xfs: sync lazy sb accounting on quiesce of read-only mounts"
> > >
> > >  _require_scratch
> > > diff --git a/tests/generic/649 b/tests/generic/649
> > > index a33b13ea..58ef96a8 100755
> > > --- a/tests/generic/649
> > > +++ b/tests/generic/649
> > > @@ -31,7 +31,7 @@ _cleanup()
> > >
> > >
> > >  # Modify as appropriate.
> > > -_fixed_by_kernel_commit 72a048c1056a \
> > > +[ "$FSTYP" = "xfs" ] && _fixed_by_kernel_commit 72a048c1056a \
> > >       "xfs: only set IOMAP_F_SHARED when providing a srcmap to a write"
> > >
> > >  _require_cp_reflink
> > > diff --git a/tests/generic/695 b/tests/generic/695
> > > index df81fdb7..694f4245 100755
> > > --- a/tests/generic/695
> > > +++ b/tests/generic/695
> > > @@ -25,7 +25,7 @@ _cleanup()
> > >  . ./common/dmflakey
> > >  . ./common/punch
> > >
> > > -_fixed_by_kernel_commit e6e3dec6c3c288 \
> > > +[ "$FSTYP" = "btrfs" ] && _fixed_by_kernel_commit e6e3dec6c3c288 \
> > >          "btrfs: update generation of hole file extent item when merging holes"
> > >  _require_scratch
> > >  _require_dm_target flakey
> > > diff --git a/tests/generic/700 b/tests/generic/700
> > > index 052cfbd6..7f84df9d 100755
> > > --- a/tests/generic/700
> > > +++ b/tests/generic/700
> > > @@ -19,8 +19,8 @@ _require_scratch
> > >  _require_attrs
> > >  _require_renameat2 whiteout
> > >
> > > -_fixed_by_kernel_commit 70b589a37e1a \
> > > -     xfs: add selinux labels to whiteout inodes
> > > +[ "$FSTYP" = "xfs" ] && _fixed_by_kernel_commit 70b589a37e1a \
> > > +     "xfs: add selinux labels to whiteout inodes"
> > >
> > >  get_selinux_label()
> > >  {
> > > diff --git a/tests/generic/701 b/tests/generic/701
> > > index 527bba34..806cc65d 100755
> > > --- a/tests/generic/701
> > > +++ b/tests/generic/701
> > > @@ -22,7 +22,7 @@ _cleanup()
> > >       rm -r -f $tmp.* $junk_dir
> > >  }
> > >
> > > -_fixed_by_kernel_commit 92fba084b79e \
> > > +[ "$FSTYP" = "exfat" ] && _fixed_by_kernel_commit 92fba084b79e \
> > >       "exfat: fix i_blocks for files truncated over 4 GiB"
> > >
> > >  _require_test
> > > diff --git a/tests/generic/702 b/tests/generic/702
> > > index a506e07d..ae47eb27 100755
> > > --- a/tests/generic/702
> > > +++ b/tests/generic/702
> > > @@ -14,7 +14,7 @@ _begin_fstest auto quick clone fiemap
> > >  . ./common/filter
> > >  . ./common/reflink
> > >
> > > -_fixed_by_kernel_commit ac3c0d36a2a2f7 \
> > > +[ "$FSTYP" = "btrfs" ] && _fixed_by_kernel_commit ac3c0d36a2a2f7 \
> > >       "btrfs: make fiemap more efficient and accurate reporting extent sharedness"
> > >
> > >  _require_scratch_reflink
> > > diff --git a/tests/generic/704 b/tests/generic/704
> > > index f452f9e9..f2360c42 100755
> > > --- a/tests/generic/704
> > > +++ b/tests/generic/704
> > > @@ -21,7 +21,9 @@ _cleanup()
> > >  # Import common functions.
> > >  . ./common/scsi_debug
> > >
> > > -_fixed_by_kernel_commit 7c71ee78031c "xfs: allow logical-sector sized O_DIRECT"
> > > +[ "$FSTYP" = "xfs" ] && _fixed_by_kernel_commit 7c71ee78031c \
> > > +     "xfs: allow logical-sector sized O_DIRECT"
> > > +
> > >  _require_scsi_debug
> > >  # If TEST_DEV is block device, make sure current fs is a localfs which can be
> > >  # written on scsi_debug device
> > > diff --git a/tests/generic/707 b/tests/generic/707
> > > index 3d8fac4b..23864038 100755
> > > --- a/tests/generic/707
> > > +++ b/tests/generic/707
> > > @@ -13,9 +13,9 @@ _begin_fstest auto
> > >
> > >  _require_scratch
> > >
> > > -_fixed_by_kernel_commit f950fd052913 \
> > > +[ "$FSTYP" = "udf" ] && _fixed_by_kernel_commit f950fd052913 \
> > >       "udf: Protect rename against modification of moved directory"
> > > -_fixed_by_kernel_commit 0813299c586b \
> > > +[ "$FSTYP" = "ext4" ] && _fixed_by_kernel_commit 0813299c586b \
> >
> > I'm wondering if it's a "ext4 only" bug, or it might can be [[ "$FSTYP" =~ ext[0-9]+ ]] ?
> 
> Not sure either, but other generic test cases do that, so it's
> probably best to do it like that.
> Do you want a new patch version with that change or can you change it yourself?

I'll change that when I merge it, just double check with you :)

Thanks,
Zorro

> 
> Thanks.
> 
> > Others looks good to me.
> >
> > Thanks,
> > Zorro
> >
> > >       "ext4: Fix possible corruption when moving a directory"
> > >
> > >  _scratch_mkfs >>$seqres.full 2>&1
> > > --
> > > 2.45.2
> > >
> > >
> >
>

Patch

diff --git a/tests/generic/365 b/tests/generic/365
index 1f6a618a..1bca848a 100755
--- a/tests/generic/365
+++ b/tests/generic/365
@@ -9,10 +9,12 @@ 
 . ./common/preamble
 _begin_fstest auto rmap fsmap
 
-_fixed_by_kernel_commit 68415b349f3f \
-	"xfs: Fix the owner setting issue for rmap query in xfs fsmap"
-_fixed_by_kernel_commit ca6448aed4f1 \
-	"xfs: Fix missing interval for missing_owner in xfs fsmap"
+if [ "$FSTYP" = "xfs" ]; then
+	_fixed_by_kernel_commit 68415b349f3f \
+		"xfs: Fix the owner setting issue for rmap query in xfs fsmap"
+	_fixed_by_kernel_commit ca6448aed4f1 \
+		"xfs: Fix missing interval for missing_owner in xfs fsmap"
+fi
 
 . ./common/filter
 
diff --git a/tests/generic/366 b/tests/generic/366
index b322bcca..b2c2e607 100755
--- a/tests/generic/366
+++ b/tests/generic/366
@@ -23,7 +23,7 @@  _require_scratch
 _require_odirect 512	# see fio job1 config below
 _require_aio
 
-_fixed_by_kernel_commit xxxxxxxxxxxx \
+[ "$FSTYP" = "btrfs" ] && _fixed_by_kernel_commit xxxxxxxxxxxx \
 	"btrfs: avoid deadlock when reading a partial uptodate folio"
 
 iterations=$((32 * LOAD_FACTOR))
diff --git a/tests/generic/367 b/tests/generic/367
index 7cf90695..ed371a02 100755
--- a/tests/generic/367
+++ b/tests/generic/367
@@ -17,7 +17,7 @@ 
 
 _begin_fstest ioctl quick
 
-_fixed_by_kernel_commit 2a492ff66673 \
+[ "$FSTYP" = "xfs" ] && _fixed_by_kernel_commit 2a492ff66673 \
 	"xfs: Check for delayed allocations before setting extsize"
 
 _require_scratch_extsize
diff --git a/tests/generic/623 b/tests/generic/623
index 6487ccb8..9f41b5cc 100755
--- a/tests/generic/623
+++ b/tests/generic/623
@@ -11,7 +11,7 @@  _begin_fstest auto quick shutdown
 
 . ./common/filter
 
-_fixed_by_kernel_commit e4826691cc7e \
+[ "$FSTYP" = "xfs" ] && _fixed_by_kernel_commit e4826691cc7e \
 	"xfs: restore shutdown check in mapped write fault path"
 
 _require_scratch_nocheck
diff --git a/tests/generic/631 b/tests/generic/631
index 8e2cf9c6..c38ab771 100755
--- a/tests/generic/631
+++ b/tests/generic/631
@@ -41,7 +41,7 @@  _require_attrs trusted
 _exclude_fs overlay
 _require_extra_fs overlay
 
-_fixed_by_kernel_commit 6da1b4b1ab36 \
+[ "$FSTYP" = "xfs" ] && _fixed_by_kernel_commit 6da1b4b1ab36 \
 	"xfs: fix an ABBA deadlock in xfs_rename"
 
 _scratch_mkfs >> $seqres.full
diff --git a/tests/generic/646 b/tests/generic/646
index dc73aeb3..b3b0ab0a 100755
--- a/tests/generic/646
+++ b/tests/generic/646
@@ -14,7 +14,7 @@ 
 . ./common/preamble
 _begin_fstest auto quick recoveryloop shutdown
 
-_fixed_by_kernel_commit 50d25484bebe \
+[ "$FSTYP" = "xfs" ] && _fixed_by_kernel_commit 50d25484bebe \
 	"xfs: sync lazy sb accounting on quiesce of read-only mounts"
 
 _require_scratch
diff --git a/tests/generic/649 b/tests/generic/649
index a33b13ea..58ef96a8 100755
--- a/tests/generic/649
+++ b/tests/generic/649
@@ -31,7 +31,7 @@  _cleanup()
 
 
 # Modify as appropriate.
-_fixed_by_kernel_commit 72a048c1056a \
+[ "$FSTYP" = "xfs" ] && _fixed_by_kernel_commit 72a048c1056a \
 	"xfs: only set IOMAP_F_SHARED when providing a srcmap to a write"
 
 _require_cp_reflink
diff --git a/tests/generic/695 b/tests/generic/695
index df81fdb7..694f4245 100755
--- a/tests/generic/695
+++ b/tests/generic/695
@@ -25,7 +25,7 @@  _cleanup()
 . ./common/dmflakey
 . ./common/punch
 
-_fixed_by_kernel_commit e6e3dec6c3c288 \
+[ "$FSTYP" = "btrfs" ] && _fixed_by_kernel_commit e6e3dec6c3c288 \
         "btrfs: update generation of hole file extent item when merging holes"
 _require_scratch
 _require_dm_target flakey
diff --git a/tests/generic/700 b/tests/generic/700
index 052cfbd6..7f84df9d 100755
--- a/tests/generic/700
+++ b/tests/generic/700
@@ -19,8 +19,8 @@  _require_scratch
 _require_attrs
 _require_renameat2 whiteout
 
-_fixed_by_kernel_commit 70b589a37e1a \
-	xfs: add selinux labels to whiteout inodes
+[ "$FSTYP" = "xfs" ] && _fixed_by_kernel_commit 70b589a37e1a \
+	"xfs: add selinux labels to whiteout inodes"
 
 get_selinux_label()
 {
diff --git a/tests/generic/701 b/tests/generic/701
index 527bba34..806cc65d 100755
--- a/tests/generic/701
+++ b/tests/generic/701
@@ -22,7 +22,7 @@  _cleanup()
 	rm -r -f $tmp.* $junk_dir
 }
 
-_fixed_by_kernel_commit 92fba084b79e \
+[ "$FSTYP" = "exfat" ] && _fixed_by_kernel_commit 92fba084b79e \
 	"exfat: fix i_blocks for files truncated over 4 GiB"
 
 _require_test
diff --git a/tests/generic/702 b/tests/generic/702
index a506e07d..ae47eb27 100755
--- a/tests/generic/702
+++ b/tests/generic/702
@@ -14,7 +14,7 @@  _begin_fstest auto quick clone fiemap
 . ./common/filter
 . ./common/reflink
 
-_fixed_by_kernel_commit ac3c0d36a2a2f7 \
+[ "$FSTYP" = "btrfs" ] && _fixed_by_kernel_commit ac3c0d36a2a2f7 \
 	"btrfs: make fiemap more efficient and accurate reporting extent sharedness"
 
 _require_scratch_reflink
diff --git a/tests/generic/704 b/tests/generic/704
index f452f9e9..f2360c42 100755
--- a/tests/generic/704
+++ b/tests/generic/704
@@ -21,7 +21,9 @@  _cleanup()
 # Import common functions.
 . ./common/scsi_debug
 
-_fixed_by_kernel_commit 7c71ee78031c "xfs: allow logical-sector sized O_DIRECT"
+[ "$FSTYP" = "xfs" ] && _fixed_by_kernel_commit 7c71ee78031c \
+	"xfs: allow logical-sector sized O_DIRECT"
+
 _require_scsi_debug
 # If TEST_DEV is block device, make sure current fs is a localfs which can be
 # written on scsi_debug device
diff --git a/tests/generic/707 b/tests/generic/707
index 3d8fac4b..23864038 100755
--- a/tests/generic/707
+++ b/tests/generic/707
@@ -13,9 +13,9 @@  _begin_fstest auto
 
 _require_scratch
 
-_fixed_by_kernel_commit f950fd052913 \
+[ "$FSTYP" = "udf" ] && _fixed_by_kernel_commit f950fd052913 \
 	"udf: Protect rename against modification of moved directory"
-_fixed_by_kernel_commit 0813299c586b \
+[ "$FSTYP" = "ext4" ] && _fixed_by_kernel_commit 0813299c586b \
 	"ext4: Fix possible corruption when moving a directory"
 
 _scratch_mkfs >>$seqres.full 2>&1