| Message ID | 20240730075653.3473323-1-maxj.fnst@fujitsu.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | xfs/348: add _fixed_by tag | expand |
On Tue, Jul 30, 2024 at 03:56:53PM +0800, Ma Xinjian wrote: > This test requires a kernel patch since 3bf963a6c6 ("xfs/348: partially revert > dbcc549317"), so note that in the test. > > Signed-off-by: Ma Xinjian <maxj.fnst@fujitsu.com> > --- > tests/xfs/348 | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/tests/xfs/348 b/tests/xfs/348 > index 3502605c..e4bc1328 100755 > --- a/tests/xfs/348 > +++ b/tests/xfs/348 > @@ -12,6 +12,9 @@ > . ./common/preamble > _begin_fstest auto quick fuzzers repair > > +_fixed_by_git_commit kernel 38de567906d95 \ > + "xfs: allow symlinks with short remote targets" Considering that 38de567906d95 is itself a fix for 1eb70f54c445f, do we want a _broken_by_git_commit to warn people not to apply 1eb70 without also applying 38de5? --D > + > # Import common functions. > . ./common/filter > . ./common/repair > -- > 2.42.0 > >
> On Tue, Jul 30, 2024 at 03:56:53PM +0800, Ma Xinjian wrote: > > This test requires a kernel patch since 3bf963a6c6 ("xfs/348: > > partially revert dbcc549317"), so note that in the test. > > > > Signed-off-by: Ma Xinjian <maxj.fnst@fujitsu.com> > > --- > > tests/xfs/348 | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/tests/xfs/348 b/tests/xfs/348 index 3502605c..e4bc1328 > > 100755 > > --- a/tests/xfs/348 > > +++ b/tests/xfs/348 > > @@ -12,6 +12,9 @@ > > . ./common/preamble > > _begin_fstest auto quick fuzzers repair > > > > +_fixed_by_git_commit kernel 38de567906d95 \ > > + "xfs: allow symlinks with short remote targets" > > Considering that 38de567906d95 is itself a fix for 1eb70f54c445f, do we want a > _broken_by_git_commit to warn people not to apply 1eb70 without also > applying 38de5? Some released Linux OS should have applied 1eb70, so for the test on these OS, the error is indeed caused by the lack of 38de5. IMO, it should be rare to apply 38de5 without applying 1eb70. So, It will be OK to use _fixed_by_git_commit. What do you think? > > --D > > > + > > # Import common functions. > > . ./common/filter > > . ./common/repair > > -- > > 2.42.0 > > > >
On Tue, Jul 30, 2024 at 07:47:51AM -0700, Darrick J. Wong wrote: > On Tue, Jul 30, 2024 at 03:56:53PM +0800, Ma Xinjian wrote: > > This test requires a kernel patch since 3bf963a6c6 ("xfs/348: partially revert > > dbcc549317"), so note that in the test. > > > > Signed-off-by: Ma Xinjian <maxj.fnst@fujitsu.com> > > --- > > tests/xfs/348 | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/tests/xfs/348 b/tests/xfs/348 > > index 3502605c..e4bc1328 100755 > > --- a/tests/xfs/348 > > +++ b/tests/xfs/348 > > @@ -12,6 +12,9 @@ > > . ./common/preamble > > _begin_fstest auto quick fuzzers repair > > > > +_fixed_by_git_commit kernel 38de567906d95 \ > > + "xfs: allow symlinks with short remote targets" > > Considering that 38de567906d95 is itself a fix for 1eb70f54c445f, do we > want a _broken_by_git_commit to warn people not to apply 1eb70 without > also applying 38de5? We already have _wants_xxxx_commit and _fixed_by_xxxx_commit, for now, I don't think we need a new one. Maybe: _fixed_by_kernel_commit 38de567906d95 .............. _wants_kernel_commit 1eb70f54c445f ............. make sense? And use some comments to explain why 1eb70 is wanted? Thanks, Zorro > > --D > > > + > > # Import common functions. > > . ./common/filter > > . ./common/repair > > -- > > 2.42.0 > > > > >
On Tue, Aug 06, 2024 at 09:19:03PM +0800, Zorro Lang wrote: > On Tue, Jul 30, 2024 at 07:47:51AM -0700, Darrick J. Wong wrote: > > On Tue, Jul 30, 2024 at 03:56:53PM +0800, Ma Xinjian wrote: > > > This test requires a kernel patch since 3bf963a6c6 ("xfs/348: partially revert > > > dbcc549317"), so note that in the test. > > > > > > Signed-off-by: Ma Xinjian <maxj.fnst@fujitsu.com> > > > --- > > > tests/xfs/348 | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/tests/xfs/348 b/tests/xfs/348 > > > index 3502605c..e4bc1328 100755 > > > --- a/tests/xfs/348 > > > +++ b/tests/xfs/348 > > > @@ -12,6 +12,9 @@ > > > . ./common/preamble > > > _begin_fstest auto quick fuzzers repair > > > > > > +_fixed_by_git_commit kernel 38de567906d95 \ > > > + "xfs: allow symlinks with short remote targets" > > > > Considering that 38de567906d95 is itself a fix for 1eb70f54c445f, do we > > want a _broken_by_git_commit to warn people not to apply 1eb70 without > > also applying 38de5? > > We already have _wants_xxxx_commit and _fixed_by_xxxx_commit, for now, I > don't think we need a new one. Maybe: > > _fixed_by_kernel_commit 38de567906d95 .............. > _wants_kernel_commit 1eb70f54c445f ............. > > make sense? And use some comments to explain why 1eb70 is wanted? Oh! I didn't realize we had _wants_kernel_commit. Yeah, that's fine. --D > Thanks, > Zorro > > > > > --D > > > > > + > > > # Import common functions. > > > . ./common/filter > > > . ./common/repair > > > -- > > > 2.42.0 > > > > > > > > > >
> On Tue, Aug 06, 2024 at 09:19:03PM +0800, Zorro Lang wrote: > > On Tue, Jul 30, 2024 at 07:47:51AM -0700, Darrick J. Wong wrote: > > > On Tue, Jul 30, 2024 at 03:56:53PM +0800, Ma Xinjian wrote: > > > > This test requires a kernel patch since 3bf963a6c6 ("xfs/348: > > > > partially revert dbcc549317"), so note that in the test. > > > > > > > > Signed-off-by: Ma Xinjian <maxj.fnst@fujitsu.com> > > > > --- > > > > tests/xfs/348 | 3 +++ > > > > 1 file changed, 3 insertions(+) > > > > > > > > diff --git a/tests/xfs/348 b/tests/xfs/348 index > > > > 3502605c..e4bc1328 100755 > > > > --- a/tests/xfs/348 > > > > +++ b/tests/xfs/348 > > > > @@ -12,6 +12,9 @@ > > > > . ./common/preamble > > > > _begin_fstest auto quick fuzzers repair > > > > > > > > +_fixed_by_git_commit kernel 38de567906d95 \ > > > > + "xfs: allow symlinks with short remote targets" > > > > > > Considering that 38de567906d95 is itself a fix for 1eb70f54c445f, do > > > we want a _broken_by_git_commit to warn people not to apply 1eb70 > > > without also applying 38de5? > > > > We already have _wants_xxxx_commit and _fixed_by_xxxx_commit, for now, > > I don't think we need a new one. Maybe: > > > > _fixed_by_kernel_commit 38de567906d95 .............. > > _wants_kernel_commit 1eb70f54c445f ............. > > > > make sense? And use some comments to explain why 1eb70 is wanted? > > Oh! I didn't realize we had _wants_kernel_commit. Yeah, that's fine. Hi Darrick Sorry, I still don't quite understand your intention. Considering that 38de567906d95 is a fix for 1eb70f54c445f, I think the current xfs/348 test should have the following 3 situations: 1. Neither 1eb70f54c445f nor 38de567906d95 are applied in the kernel: xfs/348 passes 2. Only 1eb70f54c445f is applied in the kernel without 38de567906d95: xfs/348 fails 3. Both 1eb70f54c445f and 1eb70f54c445f are applied in the kernel: xfs/348 passes The situation of " Only 38de567906d95 is applied in the kernel without 1eb70f54c445f " should not exist. Since only the second case fails, I think it's sufficient to just point out that 38de567906d95 might be missing using "_fixed_by_kernel_commit ". If my understanding is wrong, feel free to correct me, thank you very much. Best regards Ma > > --D > > > Thanks, > > Zorro > > > > > > > > --D > > > > > > > + > > > > # Import common functions. > > > > . ./common/filter > > > > . ./common/repair > > > > -- > > > > 2.42.0 > > > > > > > > > > > > > > >
On Thu, Aug 08, 2024 at 08:39:10AM +0000, Xinjian Ma (Fujitsu) wrote: > > On Tue, Aug 06, 2024 at 09:19:03PM +0800, Zorro Lang wrote: > > > On Tue, Jul 30, 2024 at 07:47:51AM -0700, Darrick J. Wong wrote: > > > > On Tue, Jul 30, 2024 at 03:56:53PM +0800, Ma Xinjian wrote: > > > > > This test requires a kernel patch since 3bf963a6c6 ("xfs/348: > > > > > partially revert dbcc549317"), so note that in the test. > > > > > > > > > > Signed-off-by: Ma Xinjian <maxj.fnst@fujitsu.com> > > > > > --- > > > > > tests/xfs/348 | 3 +++ > > > > > 1 file changed, 3 insertions(+) > > > > > > > > > > diff --git a/tests/xfs/348 b/tests/xfs/348 index > > > > > 3502605c..e4bc1328 100755 > > > > > --- a/tests/xfs/348 > > > > > +++ b/tests/xfs/348 > > > > > @@ -12,6 +12,9 @@ > > > > > . ./common/preamble > > > > > _begin_fstest auto quick fuzzers repair > > > > > > > > > > +_fixed_by_git_commit kernel 38de567906d95 \ > > > > > + "xfs: allow symlinks with short remote targets" > > > > > > > > Considering that 38de567906d95 is itself a fix for 1eb70f54c445f, do > > > > we want a _broken_by_git_commit to warn people not to apply 1eb70 > > > > without also applying 38de5? > > > > > > We already have _wants_xxxx_commit and _fixed_by_xxxx_commit, for now, > > > I don't think we need a new one. Maybe: > > > > > > _fixed_by_kernel_commit 38de567906d95 .............. > > > _wants_kernel_commit 1eb70f54c445f ............. > > > > > > make sense? And use some comments to explain why 1eb70 is wanted? > > > > Oh! I didn't realize we had _wants_kernel_commit. Yeah, that's fine. > > > Hi Darrick > > Sorry, I still don't quite understand your intention. > Considering that 38de567906d95 is a fix for 1eb70f54c445f, I think the current xfs/348 test should have the following 3 situations: > 1. Neither 1eb70f54c445f nor 38de567906d95 are applied in the kernel: xfs/348 passes > 2. Only 1eb70f54c445f is applied in the kernel without 38de567906d95: xfs/348 fails > 3. Both 1eb70f54c445f and 1eb70f54c445f are applied in the kernel: xfs/348 passes > The situation of " Only 38de567906d95 is applied in the kernel without 1eb70f54c445f " should not exist. > > Since only the second case fails, I think it's sufficient to just point out that 38de567906d95 might be missing using "_fixed_by_kernel_commit ". > If my understanding is wrong, feel free to correct me, thank you very much. 1eb70f54c445f was a bugfix for a null pointer dereference due to insufficient validation, so we really /do/ want to nudge kernel distributors to add it (and 38de567906d95) to their kernels if they don't have either. But I see your point that xfs/348 will pass without either applied, so that's not much of a nudge. In the end, I'd rather this went in with annotations for both commits, but if Zorro decides that only noting 38de567906d95 is ok, then I'll go along with that too. --D > Best regards > Ma > > > > --D > > > > > Thanks, > > > Zorro > > > > > > > > > > > --D > > > > > > > > > + > > > > > # Import common functions. > > > > > . ./common/filter > > > > > . ./common/repair > > > > > -- > > > > > 2.42.0 > > > > > > > > > > > > > > > > > > > >
> On Thu, Aug 08, 2024 at 08:39:10AM +0000, Xinjian Ma (Fujitsu) wrote: > > > On Tue, Aug 06, 2024 at 09:19:03PM +0800, Zorro Lang wrote: > > > > On Tue, Jul 30, 2024 at 07:47:51AM -0700, Darrick J. Wong wrote: > > > > > On Tue, Jul 30, 2024 at 03:56:53PM +0800, Ma Xinjian wrote: > > > > > > This test requires a kernel patch since 3bf963a6c6 ("xfs/348: > > > > > > partially revert dbcc549317"), so note that in the test. > > > > > > > > > > > > Signed-off-by: Ma Xinjian <maxj.fnst@fujitsu.com> > > > > > > --- > > > > > > tests/xfs/348 | 3 +++ > > > > > > 1 file changed, 3 insertions(+) > > > > > > > > > > > > diff --git a/tests/xfs/348 b/tests/xfs/348 index > > > > > > 3502605c..e4bc1328 100755 > > > > > > --- a/tests/xfs/348 > > > > > > +++ b/tests/xfs/348 > > > > > > @@ -12,6 +12,9 @@ > > > > > > . ./common/preamble > > > > > > _begin_fstest auto quick fuzzers repair > > > > > > > > > > > > +_fixed_by_git_commit kernel 38de567906d95 \ > > > > > > + "xfs: allow symlinks with short remote targets" > > > > > > > > > > Considering that 38de567906d95 is itself a fix for > > > > > 1eb70f54c445f, do we want a _broken_by_git_commit to warn people > > > > > not to apply 1eb70 without also applying 38de5? > > > > > > > > We already have _wants_xxxx_commit and _fixed_by_xxxx_commit, for > > > > now, I don't think we need a new one. Maybe: > > > > > > > > _fixed_by_kernel_commit 38de567906d95 .............. > > > > _wants_kernel_commit 1eb70f54c445f ............. > > > > > > > > make sense? And use some comments to explain why 1eb70 is wanted? > > > > > > Oh! I didn't realize we had _wants_kernel_commit. Yeah, that's fine. > > > > > > Hi Darrick > > > > Sorry, I still don't quite understand your intention. > > Considering that 38de567906d95 is a fix for 1eb70f54c445f, I think the current > xfs/348 test should have the following 3 situations: > > 1. Neither 1eb70f54c445f nor 38de567906d95 are applied in the kernel: > > xfs/348 passes 2. Only 1eb70f54c445f is applied in the kernel without > > 38de567906d95: xfs/348 fails 3. Both 1eb70f54c445f and 1eb70f54c445f > > are applied in the kernel: xfs/348 passes The situation of " Only > 38de567906d95 is applied in the kernel without 1eb70f54c445f " should not > exist. > > > > Since only the second case fails, I think it's sufficient to just point out that > 38de567906d95 might be missing using "_fixed_by_kernel_commit ". > > If my understanding is wrong, feel free to correct me, thank you very much. > > 1eb70f54c445f was a bugfix for a null pointer dereference due to insufficient > validation, so we really /do/ want to nudge kernel distributors to add it (and > 38de567906d95) to their kernels if they don't have either. > > But I see your point that xfs/348 will pass without either applied, so that's not > much of a nudge. In the end, I'd rather this went in with annotations for both > commits, but if Zorro decides that only noting > 38de567906d95 is ok, then I'll go along with that too. Hi Darrick Thank you for the explanation. I understand your considerations now. Sorry, I only considered whether xfs/348 passed. I have submitted [PATCH v2] xfs/348: add helper tags. PTAL. Best regards Ma > > --D > > > Best regards > > Ma > > > > > > --D > > > > > > > Thanks, > > > > Zorro > > > > > > > > > > > > > > --D > > > > > > > > > > > + > > > > > > # Import common functions. > > > > > > . ./common/filter > > > > > > . ./common/repair > > > > > > -- > > > > > > 2.42.0 > > > > > > > > > > > > > > > > > > > > > > > > >
diff --git a/tests/xfs/348 b/tests/xfs/348 index 3502605c..e4bc1328 100755 --- a/tests/xfs/348 +++ b/tests/xfs/348 @@ -12,6 +12,9 @@ . ./common/preamble _begin_fstest auto quick fuzzers repair +_fixed_by_git_commit kernel 38de567906d95 \ + "xfs: allow symlinks with short remote targets" + # Import common functions. . ./common/filter . ./common/repair
This test requires a kernel patch since 3bf963a6c6 ("xfs/348: partially revert dbcc549317"), so note that in the test. Signed-off-by: Ma Xinjian <maxj.fnst@fujitsu.com> --- tests/xfs/348 | 3 +++ 1 file changed, 3 insertions(+)