Skip to contentSkip to conversation

fstests: few updates

PATCH In review Luis R. Rodriguez 5 participants 22 messages Dec 13, 2017 – Mar 15, 2018

lore
Share & tools
lore
#0 lore raw

cover fstests: few updates

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(-)
Signature · 1 line
2.15.0
#1 lore raw

PATCH 1/9 generic/381: use username fsgqa-381

Some systems are not allowing usernames prefixed with a number now. One can however use numbers as a postfix so use that.

---
 README                |  2 +-
 tests/generic/381     | 16 ++++++++--------
 tests/generic/381.out |  4 ++--
 3 files changed, 11 insertions(+), 11 deletions(-)
README +1 -1
index ed69332e774e..e05142be1a87 100644
@@ -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
tests/generic/381 +8 -8
index 006f0d879638..cdc29c2e029e 100755
@@ -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
 
tests/generic/381.out +2 -2
index 50a1a27b0167..cc0ee4a1bb95 100644
@@ -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 [--------]
 
Signature · 1 line
2.15.0
#2 lore raw

Re: [PATCH 1/9] generic/381: use username fsgqa-381

Quoted 15 lines · Luis R. Rodriguez

Some systems are not allowing usernames prefixed with a number now. One can however use numbers as a postfix so use that.

---
 README                |  2 +-
 tests/generic/381     | 16 ++++++++--------
 tests/generic/381.out |  4 ++--
 3 files changed, 11 insertions(+), 11 deletions(-)
README +0 -0
index ed69332e774e..e05142be1a87 100644

README _______________________ · line 23

Quoted 5 lines
README +1 -1
@@ -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

#3 lore raw

Re: [PATCH 1/9] generic/381: use username fsgqa-381

In reply to Luis R. Rodriguez
Quoted 28 lines · Luis R. Rodriguez

Some systems are not allowing usernames prefixed with a number now. One can however use numbers as a postfix so use that.

---
 README                |  2 +-
 tests/generic/381     | 16 ++++++++--------
 tests/generic/381.out |  4 ++--
 3 files changed, 11 insertions(+), 11 deletions(-)
README +1 -1
index ed69332e774e..e05142be1a87 100644
@@ -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
tests/generic/381 +0 -0
index 006f0d879638..cdc29c2e029e 100755

tests/generic/381 line 4

Quoted 2 lines
tests/generic/381 +0 -0
@@ -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.

Signature · 2 lines
Dave Chinner
[email protected]
#4 lore raw

PATCH 2/9 README: document group fsgqa is required

In reply to Luis R. Rodriguez

The group fsgqa is also required.

---
 README | 1 +
 1 file changed, 1 insertion(+)
README +1 -0
index e05142be1a87..271d7df22b56 100644
@@ -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")
 	
 ______________________
Signature · 1 line
2.15.0
#5 lore raw

PATCH 3/9 generic/group: add 304 to dedupe group

In reply to Luis R. Rodriguez
---
 tests/generic/group | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tests/generic/group +1 -1
index 6c3bb03a9973..4e2fb0f720ba 100644
@@ -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
Signature · 1 line
2.15.0
#6 lore raw

PATCH 4/9 build: update AC_PACKAGE_WANT_GDBM() and src/dbtest.c to build

In reply to Luis R. Rodriguez

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.

---
 m4/package_gdbmdev.m4 | 8 ++++++++
 src/dbtest.c          | 1 +
 2 files changed, 9 insertions(+)
m4/package_gdbmdev.m4 +8 -0
index 734a192baf4d..e96343168478 100644
@@ -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
 
src/dbtest.c +1 -0
index ec8db0b93a4e..f45db4ac2b19 100644
@@ -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
Signature · 1 line
2.15.0
#7 lore raw

Re: [PATCH 4/9] build: update AC_PACKAGE_WANT_GDBM() and src/dbtest.c to build

Quoted 8 lines · Luis R. Rodriguez

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?
Quoted 5 lines

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.

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

#8 lore raw

Re: [PATCH 4/9] build: update AC_PACKAGE_WANT_GDBM() and src/dbtest.c to build

Quoted 14 lines · Eryu Guan
Quoted 8 lines · Luis R. Rodriguez

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?
Quoted 4 lines

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.

Quoted 6 lines
Quoted 1 line

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.

Quoted 3 lines

Without this patch,
dbtest was not built on openSUSE, and was built successfully with this patch applied.

Yeap.

Quoted 1 line

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
Quoted 2 lines

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.

Quoted 3 lines

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.

Quoted 3 lines

(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

#9 lore raw

Re: [PATCH 4/9] build: update AC_PACKAGE_WANT_GDBM() and src/dbtest.c to build

Quoted 47 lines · Luis R. Rodriguez
Quoted 14 lines · Eryu Guan
Quoted 8 lines · Luis R. Rodriguez

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?
Quoted 4 lines

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.

Quoted 6 lines
Quoted 1 line

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.

Quoted 3 lines

Without this patch,
dbtest was not built on openSUSE, and was built successfully with this patch applied.

Yeap.

Quoted 1 line

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
Quoted 8 lines
/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
Quoted 10 lines
Quoted 3 lines

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.

Quoted 3 lines

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.

Quoted 8 lines

Yeah makes sense now. I think we should also document when adding a group makes sense as well.

Quoted 3 lines

(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

#10 lore raw

Re: [PATCH 4/9] build: update AC_PACKAGE_WANT_GDBM() and src/dbtest.c to build

In reply to Eryu Guan
Quoted 23 lines · Eryu Guan
Quoted 8 lines · Luis R. Rodriguez

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?
Quoted 5 lines

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.

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.

Quoted 6 lines

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

Signature · 2 lines
Jeff Mahoney
SUSE Labs
#11 lore raw

PATCH 5/9 tests/xfs/group: add group for tests which require a logdev

In reply to Luis R. Rodriguez

This should make it easy to run these separately or exclude them.

---
 tests/xfs/group | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
tests/xfs/group +2 -2
index d23006041ea2..cce98847de53 100644
@@ -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
Signature · 1 line
2.15.0
#12 lore raw

Re: [PATCH 5/9] tests/xfs/group: add group for tests which require a logdev

Quoted 1 line · Luis R. Rodriguez

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....

Quoted 11 lines
---
 tests/xfs/group | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
tests/xfs/group +0 -0
index d23006041ea2..cce98847de53 100644

tests/xfs/group line 45

Quoted 5 lines
tests/xfs/group +1 -1
@@ -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.

Signature · 2 lines
Dave Chinner
[email protected]
#13 lore raw

Re: [PATCH 5/9] tests/xfs/group: add group for tests which require a logdev

Quoted 7 lines · Dave Chinner
Quoted 1 line · Luis R. Rodriguez

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

Quoted 19 lines
Quoted 11 lines
---
 tests/xfs/group | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
tests/xfs/group +0 -0
index d23006041ea2..cce98847de53 100644

tests/xfs/group line 45

Quoted 5 lines
tests/xfs/group +1 -1
@@ -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:

Quoted 6 lines

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...

Quoted 5 lines

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

#14 lore raw

Re: [PATCH 5/9] tests/xfs/group: add group for tests which require a logdev

Quoted 13 lines · Luis R. Rodriguez
Quoted 7 lines · Dave Chinner
Quoted 1 line · Luis R. Rodriguez

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.

Quoted 8 lines

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".

Quoted 4 lines

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.

Quoted 8 lines
Quoted 5 lines

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.

Signature · 2 lines
Dave Chinner
[email protected]
#15 lore raw

Re: [PATCH 5/9] tests/xfs/group: add group for tests which require a logdev

Quoted 21 lines · Dave Chinner

You can't just run an internal log test then add couple of extra external log tests and say "external logs work fine".

Quoted 4 lines

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.

Quoted 8 lines
Quoted 5 lines

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?

Quoted 6 lines

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

#16 lore raw

PATCH 6/9 tests/ext4/group: add group for tests which require a logdev

In reply to Luis R. Rodriguez

This should make it easy to run these separately or exclude them.

---
 tests/ext4/group | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tests/ext4/group +1 -1
index 257bb646f312..6fa2f06432f3 100644
@@ -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
Signature · 1 line
2.15.0
#17 lore raw

PATCH 7/9 tests/xfs/group: add realtimedev group

In reply to Luis R. Rodriguez

This groups up tests which require an external realtime volume, ie, SCRATCH_RTDEV. This requires CONFIG_XFS_RT and not all kernels support this.

---
 tests/xfs/group | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)
tests/xfs/group +18 -18
index cce98847de53..656b65f5fe7a 100644
@@ -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
Signature · 1 line
2.15.0
#18 lore raw

PATCH 8/9 tests/xfs/group: add group for tests which require mkfs v4_5

In reply to Luis R. Rodriguez

This lets us skip these tests on newer systems.

---
 tests/xfs/group | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tests/xfs/group +1 -1
index 656b65f5fe7a..ebb1685e22eb 100644
@@ -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
Signature · 1 line
2.15.0
#19 lore raw

Re: [PATCH 8/9] tests/xfs/group: add group for tests which require mkfs v4_5

Quoted 1 line · Luis R. Rodriguez

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.

Signature · 2 lines
Dave Chinner
[email protected]
#20 lore raw

PATCH 9/9 tests/xfs/group: add injection group

In reply to Luis R. Rodriguez

XFS error injection requires CONFIG_XFS_DEBUG and not all kernels have this enabled.

---
 tests/xfs/group | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
tests/xfs/group +5 -5
index ebb1685e22eb..1df0ab32b71c 100644
@@ -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
Signature · 1 line
2.15.0
#21 lore raw

Re: [PATCH 9/9] tests/xfs/group: add injection group

Quoted 2 lines · Luis R. Rodriguez

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.

Signature · 2 lines
Dave Chinner
[email protected]