I've deployed fstests on a relatively new system and ran into a few setup
snags which can be fixed easily. Other than this I also ran into a few
issues running a few tests which can easily also be fixed.
I've added a few new groups to help avoiding running tests with a basic
section. Some of these tests are better designed to be run with a custom
section and grouping them up helps with this.
Luis R. Rodriguez (9):
generic/381: use username fsgqa-381
README: document group fsgqa is required
generic/group: add 304 to dedupe group
build: update AC_PACKAGE_WANT_GDBM() and src/dbtest.c to build
tests/xfs/group: add group for tests which require a logdev
tests/ext4/group: add group for tests which require a logdev
tests/xfs/group: add realtimedev group
tests/xfs/group: add group for tests which require mkfs v4_5
tests/xfs/group: add injection group
README | 3 ++-
m4/package_gdbmdev.m4 | 8 ++++++++
src/dbtest.c | 1 +
tests/ext4/group | 2 +-
tests/generic/381 | 16 ++++++++--------
tests/generic/381.out | 4 ++--
tests/generic/group | 2 +-
tests/xfs/group | 52 +++++++++++++++++++++++++--------------------------
8 files changed, 49 insertions(+), 39 deletions(-)
--
2.15.0
XFS error injection requires CONFIG_XFS_DEBUG and not all kernels
have this enabled.
Signed-off-by: Luis R. Rodriguez <[email protected]>
---
tests/xfs/group | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/tests/xfs/group b/tests/xfs/group
index ebb1685e22eb..1df0ab32b71c 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -54,7 +54,7 @@
054 auto quick
055 dump ioctl remote tape
056 dump ioctl auto quick
-057 auto log
+057 auto log injection
058 auto quick fuzzers
059 dump ioctl auto quick
060 dump ioctl auto quick
@@ -138,7 +138,7 @@
138 auto quick
139 auto quick clone
140 auto clone
-141 auto log metadata
+141 auto log metadata injection
142 dmapi
143 dmapi
144 dmapi
@@ -193,7 +193,7 @@
193 auto quick clone
194 rw auto
195 ioctl dump auto quick
-196 auto quick rw
+196 auto quick rw injection
197 dir auto quick
198 auto quick clone
199 mount auto quick
@@ -315,7 +315,7 @@
315 auto quick clone
316 auto quick clone
317 auto quick rmap
-318 auto quick rw
+318 auto quick rw injection
319 auto quick clone
320 auto quick clone
321 auto quick clone
@@ -430,7 +430,7 @@
430 dangerous_fuzzers dangerous_scrub dangerous_online_repair
431 auto quick dangerous
432 auto quick dir metadata
-433 auto quick attr
+433 auto quick attr injection
434 auto quick clone fsr
435 auto quick clone
436 auto quick clone fsr
--
2.15.0
This should make it easy to run these separately or exclude them.
Signed-off-by: Luis R. Rodriguez <[email protected]>
---
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
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
--
2.15.0
Signed-off-by: Luis R. Rodriguez <[email protected]>
---
tests/generic/group | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/generic/group b/tests/generic/group
index 6c3bb03a9973..4e2fb0f720ba 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -306,7 +306,7 @@
301 auto quick clone
302 auto quick clone
303 auto quick clone
-304 auto quick clone
+304 auto quick clone dedupe
305 auto quick clone
306 auto quick rw
307 auto quick acl
--
2.15.0
Some systems are not allowing usernames prefixed with a number now.
One can however use numbers as a postfix so use that.
Signed-off-by: Luis R. Rodriguez <[email protected]>
---
README | 2 +-
tests/generic/381 | 16 ++++++++--------
tests/generic/381.out | 4 ++--
3 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/README b/README
index ed69332e774e..e05142be1a87 100644
--- a/README
+++ b/README
@@ -20,7 +20,7 @@ _______________________
- run make
- run make install
- create fsgqa test user ("sudo useradd fsgqa")
-- create 123456-fsgqa test user ("sudo useradd 123456-fsgqa")
+- create fsgqa-381 test user ("sudo useradd fsgqa-381")
______________________
USING THE FSQA SUITE
diff --git a/tests/generic/381 b/tests/generic/381
index 006f0d879638..cdc29c2e029e 100755
--- a/tests/generic/381
+++ b/tests/generic/381
@@ -3,7 +3,7 @@
#
# Test xfs_quota when user or names beginning with digits.
# For example, create a 'limit' for a user or group named
-# '12345678-abcd', then query this user and group.
+# 'fsgqa-381', then query this user and group.
#
#-----------------------------------------------------------------------
# Copyright (c) 2015 Red Hat Inc. All Rights Reserved.
@@ -53,9 +53,9 @@ _require_scratch
_require_quota
_require_xfs_quota_foreign
-# need user and group named 123456-fsgqa
-_require_user 123456-fsgqa
-_require_group 123456-fsgqa
+# need user and group named fsgqa-381
+_require_user fsgqa-381
+_require_group fsgqa-381
_scratch_mkfs >/dev/null 2>&1
_qmount_option "usrquota,grpquota"
@@ -63,17 +63,17 @@ _qmount
# user test
echo "== user test =="
-$XFS_QUOTA_PROG -x -c "limit -u bsoft=100m bhard=200m 123456-fsgqa" $SCRATCH_MNT
+$XFS_QUOTA_PROG -x -c "limit -u bsoft=100m bhard=200m fsgqa-381" $SCRATCH_MNT
echo "=== quota command output ==="
-$XFS_QUOTA_PROG -c "quota -u -b -N -v 123456-fsgqa" $SCRATCH_MNT | _filter_quota
+$XFS_QUOTA_PROG -c "quota -u -b -N -v fsgqa-381" $SCRATCH_MNT | _filter_quota
echo "=== report command output ==="
$XFS_QUOTA_PROG -x -c "report -u -b -N" $SCRATCH_MNT | grep -v "^root " | _filter_quota
# group test
echo "== group test =="
-$XFS_QUOTA_PROG -x -c "limit -g bsoft=100m bhard=200m 123456-fsgqa" $SCRATCH_MNT
+$XFS_QUOTA_PROG -x -c "limit -g bsoft=100m bhard=200m fsgqa-381" $SCRATCH_MNT
echo "=== quota command output ==="
-$XFS_QUOTA_PROG -c "quota -g -b -N -v 123456-fsgqa" $SCRATCH_MNT | _filter_quota
+$XFS_QUOTA_PROG -c "quota -g -b -N -v fsgqa-381 " $SCRATCH_MNT | _filter_quota
echo "=== report command output ==="
$XFS_QUOTA_PROG -x -c "report -u -b -N" $SCRATCH_MNT | grep -v "^root " | _filter_quota
diff --git a/tests/generic/381.out b/tests/generic/381.out
index 50a1a27b0167..cc0ee4a1bb95 100644
--- a/tests/generic/381.out
+++ b/tests/generic/381.out
@@ -3,11 +3,11 @@ QA output created by 381
=== quota command output ===
SCRATCH_DEV 0 102400 204800 00 [--------] SCRATCH_MNT
=== report command output ===
-123456-fsgqa 0 102400 204800 00 [--------]
+fsgqa-381 0 102400 204800 00 [--------]
== group test ==
=== quota command output ===
SCRATCH_DEV 0 102400 204800 00 [--------] SCRATCH_MNT
=== report command output ===
-123456-fsgqa 0 102400 204800 00 [--------]
+fsgqa-381 0 102400 204800 00 [--------]
--
2.15.0
This lets us skip these tests on newer systems.
Signed-off-by: Luis R. Rodriguez <[email protected]>
---
tests/xfs/group | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/xfs/group b/tests/xfs/group
index 656b65f5fe7a..ebb1685e22eb 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -93,7 +93,7 @@
093 fuzzers
094 metadata dir ioctl auto realtimedev
095 log v2log auto
-096 mkfs v2log auto quick
+096 mkfs v2log auto quick mkfs_4_5
097 fuzzers
098 fuzzers
099 fuzzers
--
2.15.0
Modern gdbm-devel packages bundle together gdbm.h and ndbm.h.
The old m4 macro had detection support for some old gdbm libraries
but not for new ones.
We fix compilation of src/dbtest.c by making the autoconf helper
check for this new arrangement:
If both gdbm.h and gdbm.h are found define set both gdbm_ndbm_=true,
and have_db=true, and define HAVE_GDBM_H. The src/dbtest.c already
had a HAVE_GDBM_H but there was never a respective autoconf settter for
it. We can just re-use this and fix it for new arrangement.
Signed-off-by: Luis R. Rodriguez <[email protected]>
---
m4/package_gdbmdev.m4 | 8 ++++++++
src/dbtest.c | 1 +
2 files changed, 9 insertions(+)
diff --git a/m4/package_gdbmdev.m4 b/m4/package_gdbmdev.m4
index 734a192baf4d..e96343168478 100644
--- a/m4/package_gdbmdev.m4
+++ b/m4/package_gdbmdev.m4
@@ -28,6 +28,14 @@ AC_DEFUN([AC_PACKAGE_WANT_GDBM],
AC_CHECK_HEADER(gdbm/ndbm.h, [ gdbm_ndbm_=true; have_db=true ], [ gdbm_ndbm_=false; have_db=false ])
if test $gdbm_ndbm_ = true; then
AC_DEFINE(HAVE_GDBM_NDBM_H_, [1], [Define to 1 if you have the <gdbm/ndbm.h> header file.])
+ else
+ AC_CHECK_HEADER(gdbm.h, [ gdbm_ndbm_=true; have_db=true ], [ gdbm_ndbm_=false; have_db=false ])
+ AC_CHECK_HEADER(ndbm.h, [ ndbm_=true ], [ ndbm_=false ])
+ if test $gdbm_ndbm_ = true; then
+ if test $ndbm_ = true; then
+ AC_DEFINE(HAVE_GDBM_H, [1], [Define to 1 if you have both <gdbm.h> and <ndbm.h> header files.])
+ fi
+ fi
fi
fi
diff --git a/src/dbtest.c b/src/dbtest.c
index ec8db0b93a4e..f45db4ac2b19 100644
--- a/src/dbtest.c
+++ b/src/dbtest.c
@@ -24,6 +24,7 @@
#include <gdbm-ndbm.h>
#elif HAVE_GDBM_H
#include <gdbm.h>
+#include <ndbm.h>
#elif HAVE_NDBM_H
#include <ndbm.h>
#else
--
2.15.0
This groups up tests which require an external realtime volume,
ie, SCRATCH_RTDEV. This requires CONFIG_XFS_RT and not all kernels
support this.
Signed-off-by: Luis R. Rodriguez <[email protected]>
---
tests/xfs/group | 36 ++++++++++++++++++------------------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/tests/xfs/group b/tests/xfs/group
index cce98847de53..656b65f5fe7a 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -87,11 +87,11 @@
087 fuzzers
088 fuzzers
089 fuzzers
-090 rw auto
+090 rw auto realtimedev
091 fuzzers
092 other auto quick
093 fuzzers
-094 metadata dir ioctl auto
+094 metadata dir ioctl auto realtimedev
095 log v2log auto
096 mkfs v2log auto quick
097 fuzzers
@@ -128,7 +128,7 @@
128 auto quick clone fsr
129 auto quick clone
130 fuzzers clone
-131 auto quick clone
+131 auto quick clone realtimedev
132 auto quick clone
133 dangerous_fuzzers
134 dangerous_fuzzers
@@ -273,7 +273,7 @@
273 auto rmap fsmap
274 auto quick rmap fsmap
275 auto quick rmap fsmap logdev
-276 auto quick rmap fsmap
+276 auto quick rmap fsmap realtimedev
277 auto quick rmap fsmap
278 repair auto
279 auto mkfs
@@ -331,16 +331,16 @@
331 auto quick rmap clone
332 auto quick rmap clone
333 auto quick rmap
-334 auto quick rmap
-335 auto rmap
-336 auto rmap
-337 fuzzers rmap
-338 auto quick rmap
-339 auto quick rmap
-340 auto quick rmap
-341 auto quick rmap
-342 auto quick rmap
-343 auto quick rmap
+334 auto quick rmap realtimedev
+335 auto rmap realtimedev
+336 auto rmap realtimedev
+337 fuzzers rmap realtimedev
+338 auto quick rmap realtimedev
+339 auto quick rmap realtimedev
+340 auto quick rmap realtimedev
+341 auto quick rmap realtimedev
+342 auto quick rmap realtimedev
+343 auto quick rmap realtimedev
344 auto quick clone
345 auto quick clone
346 auto quick clone
@@ -403,10 +403,10 @@
403 dangerous_fuzzers dangerous_scrub dangerous_online_repair
404 dangerous_fuzzers dangerous_scrub dangerous_repair
405 dangerous_fuzzers dangerous_scrub dangerous_online_repair
-406 dangerous_fuzzers dangerous_scrub dangerous_repair
+406 dangerous_fuzzers dangerous_scrub dangerous_repair realtimedev
407 dangerous_fuzzers dangerous_scrub dangerous_online_repair
-408 dangerous_fuzzers dangerous_scrub dangerous_repair
-409 dangerous_fuzzers dangerous_scrub dangerous_online_repair
+408 dangerous_fuzzers dangerous_scrub dangerous_repair realtimedev
+409 dangerous_fuzzers dangerous_scrub dangerous_online_repair realtimedev
410 dangerous_fuzzers dangerous_scrub dangerous_repair
411 dangerous_fuzzers dangerous_scrub dangerous_online_repair
412 dangerous_fuzzers dangerous_scrub dangerous_repair
@@ -416,7 +416,7 @@
416 dangerous_fuzzers dangerous_scrub dangerous_repair
417 dangerous_fuzzers dangerous_scrub dangerous_online_repair
418 dangerous_fuzzers dangerous_scrub dangerous_repair
-419 auto quick
+419 auto quick realtimedev
420 auto quick clone dedupe
421 auto quick clone dedupe
422 dangerous_scrub dangerous_online_repair
--
2.15.0
This should make it easy to run these separately or exclude them.
Signed-off-by: Luis R. Rodriguez <[email protected]>
---
tests/ext4/group | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/ext4/group b/tests/ext4/group
index 257bb646f312..6fa2f06432f3 100644
--- a/tests/ext4/group
+++ b/tests/ext4/group
@@ -31,7 +31,7 @@
026 auto quick attr
027 auto quick fsmap
028 auto quick fsmap
-029 auto quick fsmap
+029 auto quick fsmap logdev
271 auto rw quick
301 aio auto ioctl rw stress defrag
302 aio auto ioctl rw stress defrag
--
2.15.0
The group fsgqa is also required.
Signed-off-by: Luis R. Rodriguez <[email protected]>
---
README | 1 +
1 file changed, 1 insertion(+)
diff --git a/README b/README
index e05142be1a87..271d7df22b56 100644
--- a/README
+++ b/README
@@ -20,6 +20,7 @@ _______________________
- run make
- run make install
- create fsgqa test user ("sudo useradd fsgqa")
+- create fsgqa group ("sudo groupadd fsgqa")
- create fsgqa-381 test user ("sudo useradd fsgqa-381")
______________________
--
2.15.0
On Tue, Dec 12, 2017 at 04:45:11PM -0800, Luis R. Rodriguez wrote:
> Some systems are not allowing usernames prefixed with a number now.
> One can however use numbers as a postfix so use that.
>
> Signed-off-by: Luis R. Rodriguez <[email protected]>
> ---
> README | 2 +-
> tests/generic/381 | 16 ++++++++--------
> tests/generic/381.out | 4 ++--
> 3 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/README b/README
> index ed69332e774e..e05142be1a87 100644
> --- a/README
> +++ b/README
> @@ -20,7 +20,7 @@ _______________________
> - run make
> - run make install
> - create fsgqa test user ("sudo useradd fsgqa")
> -- create 123456-fsgqa test user ("sudo useradd 123456-fsgqa")
> +- create fsgqa-381 test user ("sudo useradd fsgqa-381")
I'd suggest using the username "fsgqa2" so that other tests that want
to use a second username can do so using a more logically name username.
- Ted
On Tue, Dec 12, 2017 at 04:45:11PM -0800, Luis R. Rodriguez wrote:
> Some systems are not allowing usernames prefixed with a number now.
> One can however use numbers as a postfix so use that.
>
> Signed-off-by: Luis R. Rodriguez <[email protected]>
> ---
> README | 2 +-
> tests/generic/381 | 16 ++++++++--------
> tests/generic/381.out | 4 ++--
> 3 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/README b/README
> index ed69332e774e..e05142be1a87 100644
> --- a/README
> +++ b/README
> @@ -20,7 +20,7 @@ _______________________
> - run make
> - run make install
> - create fsgqa test user ("sudo useradd fsgqa")
> -- create 123456-fsgqa test user ("sudo useradd 123456-fsgqa")
> +- create fsgqa-381 test user ("sudo useradd fsgqa-381")
>
> ______________________
> USING THE FSQA SUITE
> diff --git a/tests/generic/381 b/tests/generic/381
> index 006f0d879638..cdc29c2e029e 100755
> --- a/tests/generic/381
> +++ b/tests/generic/381
> @@ -3,7 +3,7 @@
> #
> # Test xfs_quota when user or names beginning with digits.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This isn't a "test names with digits" test, but "test names begining
with digits" test. Changing the username to not begin with digits
invalidates the entire purpose of the test which is to ensure that
xfs_quota can differentiate between UIDs and names beginning with
numbers....
So from that perspective, NAK.
IF there are distros not allowing usernames to start with digits,
then this test needs a _requires check and to _notrun on those
systems.
Cheers,
Dave.
--
Dave Chinner
[email protected]
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 <[email protected]>
> ---
> 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.
--
Dave Chinner
[email protected]
On Tue, Dec 12, 2017 at 04:45:19PM -0800, Luis R. Rodriguez wrote:
> XFS error injection requires CONFIG_XFS_DEBUG and not all kernels
> have this enabled.
NAK. this should be automatically detected by the tests and they
_not_run when support is not available.
Cheers,
Dave.
--
Dave Chinner
[email protected]
On Tue, Dec 12, 2017 at 04:45:18PM -0800, Luis R. Rodriguez wrote:
> This lets us skip these tests on newer systems.
This already _not_runs on new systems. we do not want to be adding
one-off group descriptors to avoid tests like this - the tests
themselves already detect whether they should run or not.
xfs/096 2s ... [not run] Requires older mkfs without strict input checks: the last supported version of xfsprogs is 4.5.
Why are you trying to add groups to define tests that not_run
correctly on systems they aren't supported on?
Cheers,
Dave.
--
Dave Chinner
[email protected]
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 <[email protected]>
> > ---
> > 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
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.
--
Dave Chinner
[email protected]
On Tue, Dec 12, 2017 at 04:45:14PM -0800, Luis R. Rodriguez wrote:
> Modern gdbm-devel packages bundle together gdbm.h and ndbm.h.
> The old m4 macro had detection support for some old gdbm libraries
> but not for new ones.
>
> We fix compilation of src/dbtest.c by making the autoconf helper
> check for this new arrangement:
>
> If both gdbm.h and gdbm.h are found define set both gdbm_ndbm_=true,
^^^^^^ ndbm.h?
> and have_db=true, and define HAVE_GDBM_H. The src/dbtest.c already
> had a HAVE_GDBM_H but there was never a respective autoconf settter for
> it. We can just re-use this and fix it for new arrangement.
>
> Signed-off-by: Luis R. Rodriguez <[email protected]>
This looks fine to me.
The only system I have by hand that have both <gdbm.h> and <ndbm.h> but
not any <gdbm/[gn]dbm.h> is openSUSE Tumbleweed. Without this patch,
dbtest was not built on openSUSE, and was built successfully with this
patch applied. And dbtest is still built on RHEL6/7 and Fedora.
BTW, I'll queue patch 3 and this patch for next fstests release, while
other patches seem not necessary, I agreed with Dave that groups are not
for excluding tests, the required tools and environments should be
detected by tests and _notrun if not met. (The README change looks fine,
but it doesn't apply due to the "fsgqa-381" change, so I drop it too for
now.)
Thanks,
Eryu
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
On Thu, Dec 14, 2017 at 01:51:02PM +0800, Eryu Guan wrote:
> On Tue, Dec 12, 2017 at 04:45:14PM -0800, Luis R. Rodriguez wrote:
> > Modern gdbm-devel packages bundle together gdbm.h and ndbm.h.
> > The old m4 macro had detection support for some old gdbm libraries
> > but not for new ones.
> >
> > We fix compilation of src/dbtest.c by making the autoconf helper
> > check for this new arrangement:
> >
> > If both gdbm.h and gdbm.h are found define set both gdbm_ndbm_=true,
> ^^^^^^ ndbm.h?
> > and have_db=true, and define HAVE_GDBM_H. The src/dbtest.c already
> > had a HAVE_GDBM_H but there was never a respective autoconf settter for
> > it. We can just re-use this and fix it for new arrangement.
> >
> > Signed-off-by: Luis R. Rodriguez <[email protected]>
>
> This looks fine to me.
>
> The only system I have by hand that have both <gdbm.h> and <ndbm.h> but
> not any <gdbm/[gn]dbm.h> is openSUSE Tumbleweed.
Indeed, openSUSE and SLE releases.
> Without this patch,
> dbtest was not built on openSUSE, and was built successfully with this
> patch applied.
Yeap.
> And dbtest is still built on RHEL6/7 and Fedora.
Feel free to modify the commit log accordingly then. Curious, what packages
does Fedora/ RHEL6/7 use for the requirement here?
We just have one:
$ rpm -ql gdbm-devel-1.12-1.282.x86_64
/usr/bin/gdbm_dump
/usr/bin/gdbm_load
/usr/bin/gdbmtool
/usr/include/dbm.h
/usr/include/gdbm.h
/usr/include/ndbm.h
/usr/lib64/libgdbm.a
/usr/lib64/libgdbm.so
/usr/lib64/libgdbm_compat.a
/usr/lib64/libgdbm_compat.so
/usr/lib64/libndbm.a
/usr/lib64/libndbm.so
/usr/share/info/gdbm.info.gz
/usr/share/man/man1/gdbm_dump.1.gz
/usr/share/man/man1/gdbm_load.1.gz
/usr/share/man/man1/gdbmtool.1.gz
/usr/share/man/man3/gdbm.3.gz
> BTW, I'll queue patch 3 and this patch for next fstests release, while
> other patches seem not necessary,
I think patch 2 is fine too.
> I agreed with Dave that groups are not
> for excluding tests, the required tools and environments should be
> detected by tests and _notrun if not met.
Yeah makes sense now. I think we should also document when adding
a group makes sense as well.
> (The README change looks fine,
> but it doesn't apply due to the "fsgqa-381" change, so I drop it too for
> now.)
Feel free to modify it, its not a big deal.
Luis
On Thu, Dec 14, 2017 at 06:55:03PM +0100, Luis R. Rodriguez wrote:
> On Thu, Dec 14, 2017 at 01:51:02PM +0800, Eryu Guan wrote:
> > On Tue, Dec 12, 2017 at 04:45:14PM -0800, Luis R. Rodriguez wrote:
> > > Modern gdbm-devel packages bundle together gdbm.h and ndbm.h.
> > > The old m4 macro had detection support for some old gdbm libraries
> > > but not for new ones.
> > >
> > > We fix compilation of src/dbtest.c by making the autoconf helper
> > > check for this new arrangement:
> > >
> > > If both gdbm.h and gdbm.h are found define set both gdbm_ndbm_=true,
> > ^^^^^^ ndbm.h?
> > > and have_db=true, and define HAVE_GDBM_H. The src/dbtest.c already
> > > had a HAVE_GDBM_H but there was never a respective autoconf settter for
> > > it. We can just re-use this and fix it for new arrangement.
> > >
> > > Signed-off-by: Luis R. Rodriguez <[email protected]>
> >
> > This looks fine to me.
> >
> > The only system I have by hand that have both <gdbm.h> and <ndbm.h> but
> > not any <gdbm/[gn]dbm.h> is openSUSE Tumbleweed.
>
> Indeed, openSUSE and SLE releases.
>
> > Without this patch,
> > dbtest was not built on openSUSE, and was built successfully with this
> > patch applied.
>
> Yeap.
>
> > And dbtest is still built on RHEL6/7 and Fedora.
>
> Feel free to modify the commit log accordingly then. Curious, what packages
> does Fedora/ RHEL6/7 use for the requirement here?
>
> We just have one:
>
> $ rpm -ql gdbm-devel-1.12-1.282.x86_64
> /usr/bin/gdbm_dump
> /usr/bin/gdbm_load
> /usr/bin/gdbmtool
> /usr/include/dbm.h
> /usr/include/gdbm.h
> /usr/include/ndbm.h
> /usr/lib64/libgdbm.a
> /usr/lib64/libgdbm.so
> /usr/lib64/libgdbm_compat.a
> /usr/lib64/libgdbm_compat.so
> /usr/lib64/libndbm.a
> /usr/lib64/libndbm.so
> /usr/share/info/gdbm.info.gz
> /usr/share/man/man1/gdbm_dump.1.gz
> /usr/share/man/man1/gdbm_load.1.gz
> /usr/share/man/man1/gdbmtool.1.gz
> /usr/share/man/man3/gdbm.3.gz
gdbm-devel too, but it has gdbm/[gn]dbm.h pointing to ../[gn]dbm.h, so
there's no such problem and dbtest is building normally.
# rpm -ql gdbm-devel
/usr/include/dbm.h
/usr/include/gdbm
/usr/include/gdbm.h
/usr/include/gdbm/dbm.h
/usr/include/gdbm/gdbm.h
/usr/include/gdbm/ndbm.h
/usr/include/ndbm.h
/usr/lib64/libgdbm.so
/usr/lib64/libgdbm_compat.so
/usr/share/info/gdbm.info.gz
/usr/share/man/man3/gdbm.3.gz
>
> > BTW, I'll queue patch 3 and this patch for next fstests release, while
> > other patches seem not necessary,
>
> I think patch 2 is fine too.
>
> > I agreed with Dave that groups are not
> > for excluding tests, the required tools and environments should be
> > detected by tests and _notrun if not met.
>
> Yeah makes sense now. I think we should also document when adding
> a group makes sense as well.
>
> > (The README change looks fine,
> > but it doesn't apply due to the "fsgqa-381" change, so I drop it too for
> > now.)
>
> Feel free to modify it, its not a big deal.
OK, I'll modify on commit, thanks!
Thanks,
Eryu
On 12/14/17 12:51 AM, Eryu Guan wrote:
> On Tue, Dec 12, 2017 at 04:45:14PM -0800, Luis R. Rodriguez wrote:
>> Modern gdbm-devel packages bundle together gdbm.h and ndbm.h.
>> The old m4 macro had detection support for some old gdbm libraries
>> but not for new ones.
>>
>> We fix compilation of src/dbtest.c by making the autoconf helper
>> check for this new arrangement:
>>
>> If both gdbm.h and gdbm.h are found define set both gdbm_ndbm_=true,
> ^^^^^^ ndbm.h?
>> and have_db=true, and define HAVE_GDBM_H. The src/dbtest.c already
>> had a HAVE_GDBM_H but there was never a respective autoconf settter for
>> it. We can just re-use this and fix it for new arrangement.
>>
>> Signed-off-by: Luis R. Rodriguez <[email protected]>
>
> This looks fine to me.
>
> The only system I have by hand that have both <gdbm.h> and <ndbm.h> but
> not any <gdbm/[gn]dbm.h> is openSUSE Tumbleweed. Without this patch,
> dbtest was not built on openSUSE, and was built successfully with this
> patch applied. And dbtest is still built on RHEL6/7 and Fedora.
>
> BTW, I'll queue patch 3 and this patch for next fstests release, while
> other patches seem not necessary, I agreed with Dave that groups are not
> for excluding tests, the required tools and environments should be
> detected by tests and _notrun if not met. (The README change looks fine,
> but it doesn't apply due to the "fsgqa-381" change, so I drop it too for
> now.)
Hi guys -
This change breaks on older releases like SLES 11 where both <ndbm.h>
and <gdbm.h> define datum, so we get build failures. The failure is
new, but not because it used to pass and now doesn't. It's apparently
never built on SLES releases since we ship /usr/include/ndbm.h and then
we notrun the test that uses. Now that we're looking for gdbm.h and
find it, we attempt to build src/dbtest and fail.
This fix isn't the right solution. The problem is that we have a couple
layers of old cruft that needs to be cleaned out.
1) As Luis notes, nothing sets HAVE_GDBM_H. The thing is that there is
no version of gdbm.h that exports the NDBM interface. Further, looking
at the git history, nothing has ever set HAVE_GDBM_H. It was dead code
when it was committed initially as best I can tell.
2) openSUSE Tumbleweed doesn't need <gdbm.h> at all. It needs <ndbm.h>
and this fix works because Luis added it to the HAVE_GDBM_H stanza.
3) AC_PACKAGE_WANT_NDBM used to check for <ndbm.h> but it was a check
for IRIX and the caller was removed ages ago. It wouldn't matter if it
were called anyway since libndbm is an IRIX library. Linux, IIRC, has
never shipped a libndbm.
I'll post a few patches following this to clean it up and get it working
on SLES11.
-Jeff
--
Jeff Mahoney
SUSE Labs