[v2,4/5] shared,generic: move shared/006 to generic/
diff mbox series

Message ID 20190628225910.17018-4-tytso@mit.edu
State New
Headers show
Series
  • [v2,1/5] shared,ext4: move ext4-specific tests out of shared/
Related show

Commit Message

Theodore Y. Ts'o June 28, 2019, 10:59 p.m. UTC
The shared/006 uses _scratch_mkfs_sized to create a limited size file
system, and then creates inodes until it gets ENOSPC, and then checks
to make sure the file system is consistent.  It then remounts the file
system, removes all of the files, and makes sure the file system is
consistent afterwards.

This test was marked as only being supported on ext[234] and xfs, and
so it was in shared.  However, I've tested and this test works just
fine on btrfs, ubifs, tmpfs, and should work on all file systems that
support _scratch_mkfs_sized, since even if there isn't a fixed inode
table, the file system will eventually run out of disk space.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
Changes since v1:
   - added a new _require_scratch_inode_limits and use it for generic/558
   - added missing "_supported_fs generic" line

 common/rc                         | 11 +++++++++++
 tests/{shared/006 => generic/558} |  7 +++----
 tests/generic/558.out             |  2 ++
 tests/generic/group               |  1 +
 tests/shared/006.out              |  2 --
 tests/shared/group                |  1 -
 6 files changed, 17 insertions(+), 7 deletions(-)
 rename tests/{shared/006 => generic/558} (95%)
 create mode 100644 tests/generic/558.out
 delete mode 100644 tests/shared/006.out

Comments

Eryu Guan July 5, 2019, 8:07 a.m. UTC | #1
On Fri, Jun 28, 2019 at 06:59:09PM -0400, Theodore Ts'o wrote:
> The shared/006 uses _scratch_mkfs_sized to create a limited size file
> system, and then creates inodes until it gets ENOSPC, and then checks
> to make sure the file system is consistent.  It then remounts the file
> system, removes all of the files, and makes sure the file system is
> consistent afterwards.
> 
> This test was marked as only being supported on ext[234] and xfs, and
> so it was in shared.  However, I've tested and this test works just
> fine on btrfs, ubifs, tmpfs, and should work on all file systems that
> support _scratch_mkfs_sized, since even if there isn't a fixed inode
> table, the file system will eventually run out of disk space.
> 
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> ---
> Changes since v1:
>    - added a new _require_scratch_inode_limits and use it for generic/558
>    - added missing "_supported_fs generic" line
> 
>  common/rc                         | 11 +++++++++++
>  tests/{shared/006 => generic/558} |  7 +++----
>  tests/generic/558.out             |  2 ++
>  tests/generic/group               |  1 +
>  tests/shared/006.out              |  2 --
>  tests/shared/group                |  1 -
>  6 files changed, 17 insertions(+), 7 deletions(-)
>  rename tests/{shared/006 => generic/558} (95%)
>  create mode 100644 tests/generic/558.out
>  delete mode 100644 tests/shared/006.out
> 
> diff --git a/common/rc b/common/rc
> index 9165a6f2..8e024f04 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -4147,6 +4147,17 @@ _require_scratch_btime()
>  	_scratch_unmount
>  }
>  
> +_require_scratch_inode_limits()
> +{
> +	_require_scratch
> +	_scratch_mkfs > /dev/null 2>&1
> +	_scratch_mount
> +	if [ $(_get_free_inode $SCRATCH_MNT) -eq 0 ]; then
> +		_notrun "$FSTYP does not have a fixed number of inodes available"
> +	fi
> +	_scratch_unmount
> +}

I think testing against $TEST_DIR should be sufficient, so we could
avoid the mkfs & mount & umount SCRATCH_DEV time. i.e.

_require_inode_limits()
{
        if [ $(_get_free_inode $TEST_DIR) -eq 0 ]; then
                _notrun "$FSTYP does not have a fixed number of inodes available"
        fi
}

If this looks reasonable, I can fix it on commit.

Thanks,
Eryu

> +
>  _require_filefrag_options()
>  {
>  	_require_command "$FILEFRAG_PROG" filefrag
> diff --git a/tests/shared/006 b/tests/generic/558
> similarity index 95%
> rename from tests/shared/006
> rename to tests/generic/558
> index aa65e9a2..5807dba3 100755
> --- a/tests/shared/006
> +++ b/tests/generic/558
> @@ -2,7 +2,7 @@
>  # SPDX-License-Identifier: GPL-2.0
>  # Copyright (c) 2014 Red Hat Inc.  All Rights Reserved.
>  #
> -# FS QA Test No. shared/006
> +# FS QA Test No. generic/558
>  #
>  # Stress test fs by using up all inodes and check fs.
>  #
> @@ -42,10 +42,9 @@ create_file()
>  . ./common/filter
>  
>  # real QA test starts here
> -_supported_fs ext4 ext3 ext2 xfs
> +_supported_fs generic
>  _supported_os Linux
> -
> -_require_scratch
> +_require_scratch_inode_limits
>  
>  rm -f $seqres.full
>  echo "Silence is golden"
> diff --git a/tests/generic/558.out b/tests/generic/558.out
> new file mode 100644
> index 00000000..9a6c4e79
> --- /dev/null
> +++ b/tests/generic/558.out
> @@ -0,0 +1,2 @@
> +QA output created by 558
> +Silence is golden
> diff --git a/tests/generic/group b/tests/generic/group
> index 543c0627..8fc85b63 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -560,3 +560,4 @@
>  555 auto quick cap
>  556 auto quick casefold
>  557 auto quick log
> +558 auto enospc
> diff --git a/tests/shared/006.out b/tests/shared/006.out
> deleted file mode 100644
> index 675c1b7c..00000000
> --- a/tests/shared/006.out
> +++ /dev/null
> @@ -1,2 +0,0 @@
> -QA output created by 006
> -Silence is golden
> diff --git a/tests/shared/group b/tests/shared/group
> index 3507d7d4..2cf910bd 100644
> --- a/tests/shared/group
> +++ b/tests/shared/group
> @@ -4,7 +4,6 @@
>  # - comment line before each group is "new" description
>  #
>  002 auto metadata quick log
> -006 auto enospc
>  008 auto stress dedupe
>  009 auto stress dedupe
>  010 auto stress dedupe
> -- 
> 2.22.0
>
Theodore Y. Ts'o July 6, 2019, 3:40 a.m. UTC | #2
On Fri, Jul 05, 2019 at 04:07:57PM +0800, Eryu Guan wrote:
> > +_require_scratch_inode_limits()
> > +{
> > +	_require_scratch
> > +	_scratch_mkfs > /dev/null 2>&1
> > +	_scratch_mount
> > +	if [ $(_get_free_inode $SCRATCH_MNT) -eq 0 ]; then
> > +		_notrun "$FSTYP does not have a fixed number of inodes available"
> > +	fi
> > +	_scratch_unmount
> > +}
> 
> I think testing against $TEST_DIR should be sufficient, so we could
> avoid the mkfs & mount & umount SCRATCH_DEV time.

I was following the pattern that I saw with other similar _require
tests (for example: _require_scratch_shutdown).  I *thought* the
reason why this is was done is because if the test only uses the
SCRATCH_DEV, there's no making it a requirement that TEST_DEV be
available --- since IIRC, we do support SCRATCH_DEV being available,
but not TEST_DEV.

I personally don't use xfstests in that way --- when I run xfstests,
TEST_DEV is always available and in some cases, SCRATCH_DEV won't be
present.  But I thought that's why _require_test exists --- so that
tests can be skipped if TEST_DEV does not exist.

      	     	     		      - Ted
Eryu Guan July 7, 2019, 12:51 p.m. UTC | #3
On Fri, Jul 05, 2019 at 11:40:53PM -0400, Theodore Ts'o wrote:
> On Fri, Jul 05, 2019 at 04:07:57PM +0800, Eryu Guan wrote:
> > > +_require_scratch_inode_limits()
> > > +{
> > > +	_require_scratch
> > > +	_scratch_mkfs > /dev/null 2>&1
> > > +	_scratch_mount
> > > +	if [ $(_get_free_inode $SCRATCH_MNT) -eq 0 ]; then
> > > +		_notrun "$FSTYP does not have a fixed number of inodes available"
> > > +	fi
> > > +	_scratch_unmount
> > > +}
> > 
> > I think testing against $TEST_DIR should be sufficient, so we could
> > avoid the mkfs & mount & umount SCRATCH_DEV time.
> 
> I was following the pattern that I saw with other similar _require
> tests (for example: _require_scratch_shutdown).  I *thought* the
> reason why this is was done is because if the test only uses the
> SCRATCH_DEV, there's no making it a requirement that TEST_DEV be
> available --- since IIRC, we do support SCRATCH_DEV being available,
> but not TEST_DEV.
> 
> I personally don't use xfstests in that way --- when I run xfstests,
> TEST_DEV is always available and in some cases, SCRATCH_DEV won't be
> present.  But I thought that's why _require_test exists --- so that
> tests can be skipped if TEST_DEV does not exist.

xfstests assumes TEST_DEV is always present and SCRATCH_DEV is optional
(but recommended). _require_test not only checks if TEST_DEV is avaiable
& properly mounted but also implies we should do fsck against TEST_DEV
after test, as _require_test does "touch ${RESULT_DIR}/require_test" as
well.

Regarding to _require_scratch_xxx, that's usually because the
requirement being checked might change when mkfs with different options.
But a filesystem will always have a fixed number of inodes no matter
what mkfs options it uses (the same is true for not having a fixed
number of inodes). So in this case, I think check against
TEST_DEV/TEST_DIR should be fine.

Thanks,
Eryu

Patch
diff mbox series

diff --git a/common/rc b/common/rc
index 9165a6f2..8e024f04 100644
--- a/common/rc
+++ b/common/rc
@@ -4147,6 +4147,17 @@  _require_scratch_btime()
 	_scratch_unmount
 }
 
+_require_scratch_inode_limits()
+{
+	_require_scratch
+	_scratch_mkfs > /dev/null 2>&1
+	_scratch_mount
+	if [ $(_get_free_inode $SCRATCH_MNT) -eq 0 ]; then
+		_notrun "$FSTYP does not have a fixed number of inodes available"
+	fi
+	_scratch_unmount
+}
+
 _require_filefrag_options()
 {
 	_require_command "$FILEFRAG_PROG" filefrag
diff --git a/tests/shared/006 b/tests/generic/558
similarity index 95%
rename from tests/shared/006
rename to tests/generic/558
index aa65e9a2..5807dba3 100755
--- a/tests/shared/006
+++ b/tests/generic/558
@@ -2,7 +2,7 @@ 
 # SPDX-License-Identifier: GPL-2.0
 # Copyright (c) 2014 Red Hat Inc.  All Rights Reserved.
 #
-# FS QA Test No. shared/006
+# FS QA Test No. generic/558
 #
 # Stress test fs by using up all inodes and check fs.
 #
@@ -42,10 +42,9 @@  create_file()
 . ./common/filter
 
 # real QA test starts here
-_supported_fs ext4 ext3 ext2 xfs
+_supported_fs generic
 _supported_os Linux
-
-_require_scratch
+_require_scratch_inode_limits
 
 rm -f $seqres.full
 echo "Silence is golden"
diff --git a/tests/generic/558.out b/tests/generic/558.out
new file mode 100644
index 00000000..9a6c4e79
--- /dev/null
+++ b/tests/generic/558.out
@@ -0,0 +1,2 @@ 
+QA output created by 558
+Silence is golden
diff --git a/tests/generic/group b/tests/generic/group
index 543c0627..8fc85b63 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -560,3 +560,4 @@ 
 555 auto quick cap
 556 auto quick casefold
 557 auto quick log
+558 auto enospc
diff --git a/tests/shared/006.out b/tests/shared/006.out
deleted file mode 100644
index 675c1b7c..00000000
--- a/tests/shared/006.out
+++ /dev/null
@@ -1,2 +0,0 @@ 
-QA output created by 006
-Silence is golden
diff --git a/tests/shared/group b/tests/shared/group
index 3507d7d4..2cf910bd 100644
--- a/tests/shared/group
+++ b/tests/shared/group
@@ -4,7 +4,6 @@ 
 # - comment line before each group is "new" description
 #
 002 auto metadata quick log
-006 auto enospc
 008 auto stress dedupe
 009 auto stress dedupe
 010 auto stress dedupe