[PATCHSET 0/1] fstests: fix tests for kernel 6.1

[PATCHSET 0/1] fstests: fix tests for kernel 6.1

[Darrick J. Wong]

Hi all,

Adjust filesystem tests to accomodate bug fixes merged for kernel 6.1.

If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.

This is an extraordinary way to destroy everything.  Enjoy!
Comments and questions are, as always, welcome.

--D

fstests git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfstests-dev.git/log/?h=xfs-fixes-6.1
---
 common/rc         |   15 +++++++++++++++
 tests/xfs/122     |    5 +++++
 tests/xfs/122.out |    8 ++++----
 3 files changed, 24 insertions(+), 4 deletions(-)

[PATCH 1/1] xfs/122: fix EFI/EFD log format structure size after flex array conversion

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Adjust this test since made EFI/EFD log item format structs proper flex
arrays instead of array[1].

This adjustment was made to the kernel source tree as part of a project
to make the use of flex arrays more consistent throughout the kernel.
Converting array[1] and array[0] to array[] also avoids bugs in various
compiler ports that mishandle the array size computation.  Prior to the
introduction of xfs_ondisk.h, these miscomputations resulted in kernels
that would silently write out filesystem structures that would then not
be recognized by more mainstream systems (e.g.  x86).

OFC nearly all those reports about buggy compilers are for tiny
architectures that XFS doesn't work well on anyways, so in practice it
hasn't created any user problems (AFAIK).

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 common/rc         |   15 +++++++++++++++
 tests/xfs/122     |    5 +++++
 tests/xfs/122.out |    8 ++++----
 3 files changed, 24 insertions(+), 4 deletions(-)


diff --git a/common/rc b/common/rc
index 8060c03b7d..67bd74dc89 100644
--- a/common/rc
+++ b/common/rc
@@ -1502,6 +1502,21 @@ _fixed_by_kernel_commit()
 	_fixed_by_git_commit kernel $*
 }
 
+_wants_git_commit()
+{
+	local pkg=$1
+	shift
+
+	echo "This test wants $pkg fix:" >> $seqres.hints
+	echo "      $*" >> $seqres.hints
+	echo >> $seqres.hints
+}
+
+_wants_kernel_commit()
+{
+	_wants_git_commit kernel $*
+}
+
 _check_if_dev_already_mounted()
 {
 	local dev=$1
diff --git a/tests/xfs/122 b/tests/xfs/122
index 91083d6036..e616f1987d 100755
--- a/tests/xfs/122
+++ b/tests/xfs/122
@@ -17,6 +17,11 @@ _begin_fstest other auto quick clone realtime
 _supported_fs xfs
 _require_command "$INDENT_PROG" indent
 
+# Starting in Linux 6.1, the EFI log formats were adjusted away from using
+# single-element arrays as flex arrays.
+_wants_kernel_commit 03a7485cd701 \
+	"xfs: fix memcpy fortify errors in EFI log format copying"
+
 # filter out known changes to xfs type sizes
 _type_size_filter()
 {
diff --git a/tests/xfs/122.out b/tests/xfs/122.out
index a56cbee84f..95e53c5081 100644
--- a/tests/xfs/122.out
+++ b/tests/xfs/122.out
@@ -161,10 +161,10 @@ sizeof(xfs_disk_dquot_t) = 104
 sizeof(xfs_dq_logformat_t) = 24
 sizeof(xfs_dqblk_t) = 136
 sizeof(xfs_dsb_t) = 264
-sizeof(xfs_efd_log_format_32_t) = 28
-sizeof(xfs_efd_log_format_64_t) = 32
-sizeof(xfs_efi_log_format_32_t) = 28
-sizeof(xfs_efi_log_format_64_t) = 32
+sizeof(xfs_efd_log_format_32_t) = 16
+sizeof(xfs_efd_log_format_64_t) = 16
+sizeof(xfs_efi_log_format_32_t) = 16
+sizeof(xfs_efi_log_format_64_t) = 16
 sizeof(xfs_error_injection_t) = 8
 sizeof(xfs_exntfmt_t) = 4
 sizeof(xfs_exntst_t) = 4

Re: [PATCH 1/1] xfs/122: fix EFI/EFD log format structure size after flex array conversion

[Zorro Lang]

On Tue, Dec 20, 2022 at 04:21:42PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Adjust this test since made EFI/EFD log item format structs proper flex
> arrays instead of array[1].
> 
> This adjustment was made to the kernel source tree as part of a project
> to make the use of flex arrays more consistent throughout the kernel.
> Converting array[1] and array[0] to array[] also avoids bugs in various
> compiler ports that mishandle the array size computation.  Prior to the
> introduction of xfs_ondisk.h, these miscomputations resulted in kernels
> that would silently write out filesystem structures that would then not
> be recognized by more mainstream systems (e.g.  x86).
> 
> OFC nearly all those reports about buggy compilers are for tiny
> architectures that XFS doesn't work well on anyways, so in practice it
> hasn't created any user problems (AFAIK).
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---

This version looks good to me, thanks for all these detailed information!

Reviewed-by: Zorro Lang <zlang@redhat.com>

>  common/rc         |   15 +++++++++++++++
>  tests/xfs/122     |    5 +++++
>  tests/xfs/122.out |    8 ++++----
>  3 files changed, 24 insertions(+), 4 deletions(-)
> 
> 
> diff --git a/common/rc b/common/rc
> index 8060c03b7d..67bd74dc89 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -1502,6 +1502,21 @@ _fixed_by_kernel_commit()
>  	_fixed_by_git_commit kernel $*
>  }
>  

I'd like to give some comments to the new _wants_* helpers when I merge
it (don't need send a new version again: ), to help others know the
different usage of _wants_* and _fixed_by_*. How about below comment:

# Compare with _fixed_by_* helpers, this helper is used for un-regression
# test case, e.g. xfs/122. Or a case would like to mention a git commit
# which is not a bug fix (maybe a default behavior/format change). Then
# use this helpers.

> +_wants_git_commit()
> +{
> +	local pkg=$1
> +	shift
> +
> +	echo "This test wants $pkg fix:" >> $seqres.hints
> +	echo "      $*" >> $seqres.hints
> +	echo >> $seqres.hints
> +}
> +

# Refer to _wants_git_commit

Feel free to make it better :)

Thanks,
Zorro

> +_wants_kernel_commit()
> +{
> +	_wants_git_commit kernel $*
> +}
> +
>  _check_if_dev_already_mounted()
>  {
>  	local dev=$1
> diff --git a/tests/xfs/122 b/tests/xfs/122
> index 91083d6036..e616f1987d 100755
> --- a/tests/xfs/122
> +++ b/tests/xfs/122
> @@ -17,6 +17,11 @@ _begin_fstest other auto quick clone realtime
>  _supported_fs xfs
>  _require_command "$INDENT_PROG" indent
>  
> +# Starting in Linux 6.1, the EFI log formats were adjusted away from using
> +# single-element arrays as flex arrays.
> +_wants_kernel_commit 03a7485cd701 \
> +	"xfs: fix memcpy fortify errors in EFI log format copying"
> +
>  # filter out known changes to xfs type sizes
>  _type_size_filter()
>  {
> diff --git a/tests/xfs/122.out b/tests/xfs/122.out
> index a56cbee84f..95e53c5081 100644
> --- a/tests/xfs/122.out
> +++ b/tests/xfs/122.out
> @@ -161,10 +161,10 @@ sizeof(xfs_disk_dquot_t) = 104
>  sizeof(xfs_dq_logformat_t) = 24
>  sizeof(xfs_dqblk_t) = 136
>  sizeof(xfs_dsb_t) = 264
> -sizeof(xfs_efd_log_format_32_t) = 28
> -sizeof(xfs_efd_log_format_64_t) = 32
> -sizeof(xfs_efi_log_format_32_t) = 28
> -sizeof(xfs_efi_log_format_64_t) = 32
> +sizeof(xfs_efd_log_format_32_t) = 16
> +sizeof(xfs_efd_log_format_64_t) = 16
> +sizeof(xfs_efi_log_format_32_t) = 16
> +sizeof(xfs_efi_log_format_64_t) = 16
>  sizeof(xfs_error_injection_t) = 8
>  sizeof(xfs_exntfmt_t) = 4
>  sizeof(xfs_exntst_t) = 4
> 

Re: [PATCH 1/1] xfs/122: fix EFI/EFD log format structure size after flex array conversion

[Darrick J. Wong]

On Thu, Dec 22, 2022 at 03:19:00PM +0800, Zorro Lang wrote:
> On Tue, Dec 20, 2022 at 04:21:42PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Adjust this test since made EFI/EFD log item format structs proper flex
> > arrays instead of array[1].
> > 
> > This adjustment was made to the kernel source tree as part of a project
> > to make the use of flex arrays more consistent throughout the kernel.
> > Converting array[1] and array[0] to array[] also avoids bugs in various
> > compiler ports that mishandle the array size computation.  Prior to the
> > introduction of xfs_ondisk.h, these miscomputations resulted in kernels
> > that would silently write out filesystem structures that would then not
> > be recognized by more mainstream systems (e.g.  x86).
> > 
> > OFC nearly all those reports about buggy compilers are for tiny
> > architectures that XFS doesn't work well on anyways, so in practice it
> > hasn't created any user problems (AFAIK).
> > 
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> 
> This version looks good to me, thanks for all these detailed information!
> 
> Reviewed-by: Zorro Lang <zlang@redhat.com>
> 
> >  common/rc         |   15 +++++++++++++++
> >  tests/xfs/122     |    5 +++++
> >  tests/xfs/122.out |    8 ++++----
> >  3 files changed, 24 insertions(+), 4 deletions(-)
> > 
> > 
> > diff --git a/common/rc b/common/rc
> > index 8060c03b7d..67bd74dc89 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -1502,6 +1502,21 @@ _fixed_by_kernel_commit()
> >  	_fixed_by_git_commit kernel $*
> >  }
> >  
> 
> I'd like to give some comments to the new _wants_* helpers when I merge
> it (don't need send a new version again: ), to help others know the
> different usage of _wants_* and _fixed_by_*. How about below comment:
> 
> # Compare with _fixed_by_* helpers, this helper is used for un-regression
> # test case, e.g. xfs/122. Or a case would like to mention a git commit
> # which is not a bug fix (maybe a default behavior/format change). Then
> # use this helpers.

How about:

"For test cases that are not regression tests, e.g. functional tests or
maintainer tests, this helper suggests git commits that should be
applied to source trees to avoid test failures."

--D

> 
> > +_wants_git_commit()
> > +{
> > +	local pkg=$1
> > +	shift
> > +
> > +	echo "This test wants $pkg fix:" >> $seqres.hints
> > +	echo "      $*" >> $seqres.hints
> > +	echo >> $seqres.hints
> > +}
> > +
> 
> # Refer to _wants_git_commit
> 
> Feel free to make it better :)
> 
> Thanks,
> Zorro
> 
> > +_wants_kernel_commit()
> > +{
> > +	_wants_git_commit kernel $*
> > +}
> > +
> >  _check_if_dev_already_mounted()
> >  {
> >  	local dev=$1
> > diff --git a/tests/xfs/122 b/tests/xfs/122
> > index 91083d6036..e616f1987d 100755
> > --- a/tests/xfs/122
> > +++ b/tests/xfs/122
> > @@ -17,6 +17,11 @@ _begin_fstest other auto quick clone realtime
> >  _supported_fs xfs
> >  _require_command "$INDENT_PROG" indent
> >  
> > +# Starting in Linux 6.1, the EFI log formats were adjusted away from using
> > +# single-element arrays as flex arrays.
> > +_wants_kernel_commit 03a7485cd701 \
> > +	"xfs: fix memcpy fortify errors in EFI log format copying"
> > +
> >  # filter out known changes to xfs type sizes
> >  _type_size_filter()
> >  {
> > diff --git a/tests/xfs/122.out b/tests/xfs/122.out
> > index a56cbee84f..95e53c5081 100644
> > --- a/tests/xfs/122.out
> > +++ b/tests/xfs/122.out
> > @@ -161,10 +161,10 @@ sizeof(xfs_disk_dquot_t) = 104
> >  sizeof(xfs_dq_logformat_t) = 24
> >  sizeof(xfs_dqblk_t) = 136
> >  sizeof(xfs_dsb_t) = 264
> > -sizeof(xfs_efd_log_format_32_t) = 28
> > -sizeof(xfs_efd_log_format_64_t) = 32
> > -sizeof(xfs_efi_log_format_32_t) = 28
> > -sizeof(xfs_efi_log_format_64_t) = 32
> > +sizeof(xfs_efd_log_format_32_t) = 16
> > +sizeof(xfs_efd_log_format_64_t) = 16
> > +sizeof(xfs_efi_log_format_32_t) = 16
> > +sizeof(xfs_efi_log_format_64_t) = 16
> >  sizeof(xfs_error_injection_t) = 8
> >  sizeof(xfs_exntfmt_t) = 4
> >  sizeof(xfs_exntst_t) = 4
> > 
> 

[PATCHSET 0/1] fstests: fix tests for kernel 6.1

[Darrick J. Wong]

Hi all,

Adjust filesystem tests to accomodate bug fixes merged for kernel 6.1.

If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.

This is an extraordinary way to destroy everything.  Enjoy!
Comments and questions are, as always, welcome.

--D

fstests git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfstests-dev.git/log/?h=xfs-fixes-6.1
---
 tests/xfs/122.out |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

[PATCH 1/1] xfs/122: fix EFI/EFD log format structure size after flex array conversion

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Adjust this test since made EFI/EFD log item format structs proper flex
arrays instead of array[1].

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 tests/xfs/122.out |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)


diff --git a/tests/xfs/122.out b/tests/xfs/122.out
index a56cbee84f..95e53c5081 100644
--- a/tests/xfs/122.out
+++ b/tests/xfs/122.out
@@ -161,10 +161,10 @@ sizeof(xfs_disk_dquot_t) = 104
 sizeof(xfs_dq_logformat_t) = 24
 sizeof(xfs_dqblk_t) = 136
 sizeof(xfs_dsb_t) = 264
-sizeof(xfs_efd_log_format_32_t) = 28
-sizeof(xfs_efd_log_format_64_t) = 32
-sizeof(xfs_efi_log_format_32_t) = 28
-sizeof(xfs_efi_log_format_64_t) = 32
+sizeof(xfs_efd_log_format_32_t) = 16
+sizeof(xfs_efd_log_format_64_t) = 16
+sizeof(xfs_efi_log_format_32_t) = 16
+sizeof(xfs_efi_log_format_64_t) = 16
 sizeof(xfs_error_injection_t) = 8
 sizeof(xfs_exntfmt_t) = 4
 sizeof(xfs_exntst_t) = 4

Re: [PATCH 1/1] xfs/122: fix EFI/EFD log format structure size after flex array conversion

[Zorro Lang]

On Tue, Dec 13, 2022 at 11:45:42AM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Adjust this test since made EFI/EFD log item format structs proper flex
> arrays instead of array[1].
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---

So we let this case fail on all older system/kernel? Is the kernel patch
recommended to be merged on downstream kernel?

Thanks,
Zorro

>  tests/xfs/122.out |    8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> 
> diff --git a/tests/xfs/122.out b/tests/xfs/122.out
> index a56cbee84f..95e53c5081 100644
> --- a/tests/xfs/122.out
> +++ b/tests/xfs/122.out
> @@ -161,10 +161,10 @@ sizeof(xfs_disk_dquot_t) = 104
>  sizeof(xfs_dq_logformat_t) = 24
>  sizeof(xfs_dqblk_t) = 136
>  sizeof(xfs_dsb_t) = 264
> -sizeof(xfs_efd_log_format_32_t) = 28
> -sizeof(xfs_efd_log_format_64_t) = 32
> -sizeof(xfs_efi_log_format_32_t) = 28
> -sizeof(xfs_efi_log_format_64_t) = 32
> +sizeof(xfs_efd_log_format_32_t) = 16
> +sizeof(xfs_efd_log_format_64_t) = 16
> +sizeof(xfs_efi_log_format_32_t) = 16
> +sizeof(xfs_efi_log_format_64_t) = 16
>  sizeof(xfs_error_injection_t) = 8
>  sizeof(xfs_exntfmt_t) = 4
>  sizeof(xfs_exntst_t) = 4
> 

Re: [PATCH 1/1] xfs/122: fix EFI/EFD log format structure size after flex array conversion

[Darrick J. Wong]

On Thu, Dec 15, 2022 at 02:40:47AM +0800, Zorro Lang wrote:
> On Tue, Dec 13, 2022 at 11:45:42AM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Adjust this test since made EFI/EFD log item format structs proper flex
> > arrays instead of array[1].
> > 
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> 
> So we let this case fail on all older system/kernel? Is the kernel patch
> recommended to be merged on downstream kernel?

Yes, since there are certain buggy compilers that mishandle the array
size computation.  Prior to the introduction of xfs_ondisk.h, they were
silently writing out filesystem structures that would not be recognized
by more mainstream systems (e.g. x86).

OFC nearly all those reports about buggy compilers are for tiny
architectures that XFS doesn't work well on anyways, so in practice it
hasn't created any user problems (AFAIK).

--D

> Thanks,
> Zorro
> 
> >  tests/xfs/122.out |    8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > 
> > diff --git a/tests/xfs/122.out b/tests/xfs/122.out
> > index a56cbee84f..95e53c5081 100644
> > --- a/tests/xfs/122.out
> > +++ b/tests/xfs/122.out
> > @@ -161,10 +161,10 @@ sizeof(xfs_disk_dquot_t) = 104
> >  sizeof(xfs_dq_logformat_t) = 24
> >  sizeof(xfs_dqblk_t) = 136
> >  sizeof(xfs_dsb_t) = 264
> > -sizeof(xfs_efd_log_format_32_t) = 28
> > -sizeof(xfs_efd_log_format_64_t) = 32
> > -sizeof(xfs_efi_log_format_32_t) = 28
> > -sizeof(xfs_efi_log_format_64_t) = 32
> > +sizeof(xfs_efd_log_format_32_t) = 16
> > +sizeof(xfs_efd_log_format_64_t) = 16
> > +sizeof(xfs_efi_log_format_32_t) = 16
> > +sizeof(xfs_efi_log_format_64_t) = 16
> >  sizeof(xfs_error_injection_t) = 8
> >  sizeof(xfs_exntfmt_t) = 4
> >  sizeof(xfs_exntst_t) = 4
> > 
> 

Re: [PATCH 1/1] xfs/122: fix EFI/EFD log format structure size after flex array conversion

[Zorro Lang]

On Sat, Dec 17, 2022 at 12:14:10AM -0800, Darrick J. Wong wrote:
> On Thu, Dec 15, 2022 at 02:40:47AM +0800, Zorro Lang wrote:
> > On Tue, Dec 13, 2022 at 11:45:42AM -0800, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <djwong@kernel.org>
> > > 
> > > Adjust this test since made EFI/EFD log item format structs proper flex
> > > arrays instead of array[1].
> > > 
> > > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > > ---
> > 
> > So we let this case fail on all older system/kernel? Is the kernel patch
> > recommended to be merged on downstream kernel?
> 
> Yes, since there are certain buggy compilers that mishandle the array
> size computation.  Prior to the introduction of xfs_ondisk.h, they were
> silently writing out filesystem structures that would not be recognized
> by more mainstream systems (e.g. x86).
> 
> OFC nearly all those reports about buggy compilers are for tiny
> architectures that XFS doesn't work well on anyways, so in practice it
> hasn't created any user problems (AFAIK).

Thanks, may you provide this detailed explanation in commit log, and better
to point out the kernel commits which is related with this testing change.

Due to this case isn't a case for a known issue, I think it might be no
suitable to add _fixed_by_kernel_commit in this case, but how about giving
more details in commit log.

Thanks,
Zorro

> 
> --D
> 
> > Thanks,
> > Zorro
> > 
> > >  tests/xfs/122.out |    8 ++++----
> > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > > 
> > > 
> > > diff --git a/tests/xfs/122.out b/tests/xfs/122.out
> > > index a56cbee84f..95e53c5081 100644
> > > --- a/tests/xfs/122.out
> > > +++ b/tests/xfs/122.out
> > > @@ -161,10 +161,10 @@ sizeof(xfs_disk_dquot_t) = 104
> > >  sizeof(xfs_dq_logformat_t) = 24
> > >  sizeof(xfs_dqblk_t) = 136
> > >  sizeof(xfs_dsb_t) = 264
> > > -sizeof(xfs_efd_log_format_32_t) = 28
> > > -sizeof(xfs_efd_log_format_64_t) = 32
> > > -sizeof(xfs_efi_log_format_32_t) = 28
> > > -sizeof(xfs_efi_log_format_64_t) = 32
> > > +sizeof(xfs_efd_log_format_32_t) = 16
> > > +sizeof(xfs_efd_log_format_64_t) = 16
> > > +sizeof(xfs_efi_log_format_32_t) = 16
> > > +sizeof(xfs_efi_log_format_64_t) = 16
> > >  sizeof(xfs_error_injection_t) = 8
> > >  sizeof(xfs_exntfmt_t) = 4
> > >  sizeof(xfs_exntst_t) = 4
> > > 
> > 
> 

Re: [PATCH 1/1] xfs/122: fix EFI/EFD log format structure size after flex array conversion

[Darrick J. Wong]

On Sat, Dec 17, 2022 at 06:00:29PM +0800, Zorro Lang wrote:
> On Sat, Dec 17, 2022 at 12:14:10AM -0800, Darrick J. Wong wrote:
> > On Thu, Dec 15, 2022 at 02:40:47AM +0800, Zorro Lang wrote:
> > > On Tue, Dec 13, 2022 at 11:45:42AM -0800, Darrick J. Wong wrote:
> > > > From: Darrick J. Wong <djwong@kernel.org>
> > > > 
> > > > Adjust this test since made EFI/EFD log item format structs proper flex
> > > > arrays instead of array[1].
> > > > 
> > > > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > > > ---
> > > 
> > > So we let this case fail on all older system/kernel? Is the kernel patch
> > > recommended to be merged on downstream kernel?
> > 
> > Yes, since there are certain buggy compilers that mishandle the array
> > size computation.  Prior to the introduction of xfs_ondisk.h, they were
> > silently writing out filesystem structures that would not be recognized
> > by more mainstream systems (e.g. x86).
> > 
> > OFC nearly all those reports about buggy compilers are for tiny
> > architectures that XFS doesn't work well on anyways, so in practice it
> > hasn't created any user problems (AFAIK).
> 
> Thanks, may you provide this detailed explanation in commit log, and better
> to point out the kernel commits which is related with this testing change.

Will do.

> Due to this case isn't a case for a known issue, I think it might be no
> suitable to add _fixed_by_kernel_commit in this case, but how about giving
> more details in commit log.

Er.... xfs/122 isn't a regression test, so it's not testing previously
broken and now fixed code.  While I sense that a few peoples'
understanding of _fixed_by_kernel_commit might be constrained to "if
this test fails, check that your kernel/*fsprogs have commit XXXXX", I
myself have started `grep _fixed_by_kernel_commit' to find bug fixes and
their related regression tests to suggest backports.

...although I wonder if perhaps we should have a second set of
_by_commit helpers that encode "not a regression test, but you might
check such-and-such commit"?

--D

> Thanks,
> Zorro
> 
> > 
> > --D
> > 
> > > Thanks,
> > > Zorro
> > > 
> > > >  tests/xfs/122.out |    8 ++++----
> > > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > > > 
> > > > 
> > > > diff --git a/tests/xfs/122.out b/tests/xfs/122.out
> > > > index a56cbee84f..95e53c5081 100644
> > > > --- a/tests/xfs/122.out
> > > > +++ b/tests/xfs/122.out
> > > > @@ -161,10 +161,10 @@ sizeof(xfs_disk_dquot_t) = 104
> > > >  sizeof(xfs_dq_logformat_t) = 24
> > > >  sizeof(xfs_dqblk_t) = 136
> > > >  sizeof(xfs_dsb_t) = 264
> > > > -sizeof(xfs_efd_log_format_32_t) = 28
> > > > -sizeof(xfs_efd_log_format_64_t) = 32
> > > > -sizeof(xfs_efi_log_format_32_t) = 28
> > > > -sizeof(xfs_efi_log_format_64_t) = 32
> > > > +sizeof(xfs_efd_log_format_32_t) = 16
> > > > +sizeof(xfs_efd_log_format_64_t) = 16
> > > > +sizeof(xfs_efi_log_format_32_t) = 16
> > > > +sizeof(xfs_efi_log_format_64_t) = 16
> > > >  sizeof(xfs_error_injection_t) = 8
> > > >  sizeof(xfs_exntfmt_t) = 4
> > > >  sizeof(xfs_exntst_t) = 4
> > > > 
> > > 
> > 
> 

Re: [PATCH 1/1] xfs/122: fix EFI/EFD log format structure size after flex array conversion

[Zorro Lang]

On Mon, Dec 19, 2022 at 09:14:50AM -0800, Darrick J. Wong wrote:
> On Sat, Dec 17, 2022 at 06:00:29PM +0800, Zorro Lang wrote:
> > On Sat, Dec 17, 2022 at 12:14:10AM -0800, Darrick J. Wong wrote:
> > > On Thu, Dec 15, 2022 at 02:40:47AM +0800, Zorro Lang wrote:
> > > > On Tue, Dec 13, 2022 at 11:45:42AM -0800, Darrick J. Wong wrote:
> > > > > From: Darrick J. Wong <djwong@kernel.org>
> > > > > 
> > > > > Adjust this test since made EFI/EFD log item format structs proper flex
> > > > > arrays instead of array[1].
> > > > > 
> > > > > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > > > > ---
> > > > 
> > > > So we let this case fail on all older system/kernel? Is the kernel patch
> > > > recommended to be merged on downstream kernel?
> > > 
> > > Yes, since there are certain buggy compilers that mishandle the array
> > > size computation.  Prior to the introduction of xfs_ondisk.h, they were
> > > silently writing out filesystem structures that would not be recognized
> > > by more mainstream systems (e.g. x86).
> > > 
> > > OFC nearly all those reports about buggy compilers are for tiny
> > > architectures that XFS doesn't work well on anyways, so in practice it
> > > hasn't created any user problems (AFAIK).
> > 
> > Thanks, may you provide this detailed explanation in commit log, and better
> > to point out the kernel commits which is related with this testing change.
> 
> Will do.
> 
> > Due to this case isn't a case for a known issue, I think it might be no
> > suitable to add _fixed_by_kernel_commit in this case, but how about giving
> > more details in commit log.
> 
> Er.... xfs/122 isn't a regression test, so it's not testing previously
> broken and now fixed code.  While I sense that a few peoples'
> understanding of _fixed_by_kernel_commit might be constrained to "if
> this test fails, check that your kernel/*fsprogs have commit XXXXX", I
> myself have started `grep _fixed_by_kernel_commit' to find bug fixes and
> their related regression tests to suggest backports.

I generally check _fixed_by_*, secondly check the comments in the code and
the commit log related with the case.

> 
> ...although I wonder if perhaps we should have a second set of
> _by_commit helpers that encode "not a regression test, but you might
> check such-and-such commit"?

Yeah, the "fixed_by" sounds not precise for xfs/122. Maybe "_cover_commit" or
some better names you have :)

Thanks,
Zorro

> 
> --D
> 
> > Thanks,
> > Zorro
> > 
> > > 
> > > --D
> > > 
> > > > Thanks,
> > > > Zorro
> > > > 
> > > > >  tests/xfs/122.out |    8 ++++----
> > > > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > > > > 
> > > > > 
> > > > > diff --git a/tests/xfs/122.out b/tests/xfs/122.out
> > > > > index a56cbee84f..95e53c5081 100644
> > > > > --- a/tests/xfs/122.out
> > > > > +++ b/tests/xfs/122.out
> > > > > @@ -161,10 +161,10 @@ sizeof(xfs_disk_dquot_t) = 104
> > > > >  sizeof(xfs_dq_logformat_t) = 24
> > > > >  sizeof(xfs_dqblk_t) = 136
> > > > >  sizeof(xfs_dsb_t) = 264
> > > > > -sizeof(xfs_efd_log_format_32_t) = 28
> > > > > -sizeof(xfs_efd_log_format_64_t) = 32
> > > > > -sizeof(xfs_efi_log_format_32_t) = 28
> > > > > -sizeof(xfs_efi_log_format_64_t) = 32
> > > > > +sizeof(xfs_efd_log_format_32_t) = 16
> > > > > +sizeof(xfs_efd_log_format_64_t) = 16
> > > > > +sizeof(xfs_efi_log_format_32_t) = 16
> > > > > +sizeof(xfs_efi_log_format_64_t) = 16
> > > > >  sizeof(xfs_error_injection_t) = 8
> > > > >  sizeof(xfs_exntfmt_t) = 4
> > > > >  sizeof(xfs_exntst_t) = 4
> > > > > 
> > > > 
> > > 
> > 
>