| Message ID | 20171213004519.29340-6-mcgrof@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Dec 12, 2017 at 04:45:15PM -0800, Luis R. Rodriguez wrote: > This should make it easy to run these separately or exclude them. These should notrun automatically if you don't have an external log device configured. Every test should either work with an external logdev or explicitly notrun them, so I'm not sure what you're trying to acheive here.... > > Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org> > --- > tests/xfs/group | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/tests/xfs/group b/tests/xfs/group > index d23006041ea2..cce98847de53 100644 > --- a/tests/xfs/group > +++ b/tests/xfs/group > @@ -42,7 +42,7 @@ > 042 fsr ioctl auto > 043 dump ioctl tape > 044 other auto > -045 other auto quick > +045 other auto quick logdev This change also looks wrong because: xfs/044 [not run] This test requires a valid $SCRATCH_LOGDEV xfs/045 1s ... 1s xfs/044 is the external logdev test, and xfs/045 is a duplicate uuid mount test that has nothign to do with external log devices. And, FWIW, we already have a "log" group to indicate tests that exercise the log, and that mostly includes all the tests that use external logs. It would be better to tag all the tests that exercise the log with "log" rather than create some new group that doesn't really provide any added benefit.... Cheers, Dave.
On Thu, Dec 14, 2017 at 08:50:13AM +1100, Dave Chinner wrote: > On Tue, Dec 12, 2017 at 04:45:15PM -0800, Luis R. Rodriguez wrote: > > This should make it easy to run these separately or exclude them. > > These should notrun automatically if you don't have an external log > device configured. Every test should either work with an external > logdev or explicitly notrun them, so I'm not sure what you're trying > to acheive here.... The way I'm splitting up tests is one first run with a basic xfs section on a configuration file, with no external log, which pretty much runs all tests but excludes all which require external or funky configurations. A secondary pass then goes through these extra groups and then runs tests only for the previously excluded groups but with their own respective section. So for instance in this case I have: [xfs] .... [logdev_xfs] ... Automatic detection if the requirements are met is fine, but this doesn't let me easily use say: ./check -s logdev_xfs -g logdev > > > > Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org> > > --- > > tests/xfs/group | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/tests/xfs/group b/tests/xfs/group > > index d23006041ea2..cce98847de53 100644 > > --- a/tests/xfs/group > > +++ b/tests/xfs/group > > @@ -42,7 +42,7 @@ > > 042 fsr ioctl auto > > 043 dump ioctl tape > > 044 other auto > > -045 other auto quick > > +045 other auto quick logdev > > This change also looks wrong because: > > xfs/044 [not run] This test requires a valid $SCRATCH_LOGDEV > xfs/045 1s ... 1s > > xfs/044 is the external logdev test, and xfs/045 is a > duplicate uuid mount test that has nothign to do with external > log devices. I see... > And, FWIW, we already have a "log" group to indicate tests that > exercise the log, and that mostly includes all the tests that use > external logs. It would be better to tag all the tests that exercise > the log with "log" rather than create some new group that doesn't > really provide any added benefit.... So for my case would one better goal be to just run check without the external one and one with the external log? ./check -s xfs -g log ./check -s logdev_xfs -g log Luis -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Dec 14, 2017 at 12:00:52AM +0100, Luis R. Rodriguez wrote: > On Thu, Dec 14, 2017 at 08:50:13AM +1100, Dave Chinner wrote: > > On Tue, Dec 12, 2017 at 04:45:15PM -0800, Luis R. Rodriguez wrote: > > > This should make it easy to run these separately or exclude them. > > > > These should notrun automatically if you don't have an external log > > device configured. Every test should either work with an external > > logdev or explicitly notrun them, so I'm not sure what you're trying > > to acheive here.... > > The way I'm splitting up tests is one first run with a basic xfs section > on a configuration file, with no external log, which pretty much runs all > tests but excludes all which require external or funky configurations. > > A secondary pass then goes through these extra groups and then runs tests > only for the previously excluded groups but with their own respective > section. So for instance in this case I have: > > [xfs] > .... > [logdev_xfs] > ... Which seems to me like a misguided attempt to optimise test runtimes. i.e. this doesn't provide test coverage of external log behaviour in all the cases that need to be tested. Data integrity code paths are affected by having an external log. IO ordering changes with external logs, which can expose update/crash recovery problems. external logs can expose data IO race conditions that are masked by interleaved log IO. etc, etc, etc. You can't just run an internal log test then add couple of extra external log tests and say "external logs work fine". > Automatic detection if the requirements are met is fine, but this doesn't > let me easily use say: > > ./check -s logdev_xfs -g logdev You can do that if we ignore the fact that a large number of tests need to be run on both internal and external log devices to cover the differences in behaviour between them. > > And, FWIW, we already have a "log" group to indicate tests that > > exercise the log, and that mostly includes all the tests that use > > external logs. It would be better to tag all the tests that exercise > > the log with "log" rather than create some new group that doesn't > > really provide any added benefit.... > > So for my case would one better goal be to just run check without the external > one and one with the external log? See above. Your test coverage assumptions are wrong, so what you are trying to do really doesn't tell you whether external logs work correctly or not. It's worse that not testing external logs at all, because it gives the false impression that they have been exhaustively tested and work just fine when that really isn't the case. Cheers, Dave.
On Thu, Dec 14, 2017 at 10:39:14AM +1100, Dave Chinner wrote: > You can't just run an internal log test then add couple of extra > external log tests and say "external logs work fine". > > > Automatic detection if the requirements are met is fine, but this doesn't > > let me easily use say: > > > > ./check -s logdev_xfs -g logdev > > You can do that if we ignore the fact that a large number of tests > need to be run on both internal and external log devices to cover > the differences in behaviour between them. > > > > And, FWIW, we already have a "log" group to indicate tests that > > > exercise the log, and that mostly includes all the tests that use > > > external logs. It would be better to tag all the tests that exercise > > > the log with "log" rather than create some new group that doesn't > > > really provide any added benefit.... > > > > So for my case would one better goal be to just run check without the external > > one and one with the external log? > > See above. Your test coverage assumptions are wrong, so what you are > trying to do really doesn't tell you whether external logs work > correctly or not. It's worse that not testing external logs at all, > because it gives the false impression that they have been > exhaustively tested and work just fine when that really isn't the > case. Makes sense, thanks. Luis -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/tests/xfs/group b/tests/xfs/group index d23006041ea2..cce98847de53 100644 --- a/tests/xfs/group +++ b/tests/xfs/group @@ -42,7 +42,7 @@ 042 fsr ioctl auto 043 dump ioctl tape 044 other auto -045 other auto quick +045 other auto quick logdev 046 dump ioctl auto quick 047 dump ioctl auto 048 other auto quick @@ -272,7 +272,7 @@ 272 auto quick rmap fsmap 273 auto rmap fsmap 274 auto quick rmap fsmap -275 auto quick rmap fsmap +275 auto quick rmap fsmap logdev 276 auto quick rmap fsmap 277 auto quick rmap fsmap 278 repair auto
This should make it easy to run these separately or exclude them. Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org> --- tests/xfs/group | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)