[PATCH 1/2] fstests: make generic/500 xfs+ext4 only

[PATCH 1/2] fstests: make generic/500 xfs+ext4 only

[Josef Bacik]

I recently fixed some bugs in btrfs's enospc handling that made it start
failing generic/500.

The point of this test is to make the thin provisioned device run out of
space, which results in an EIO being seen on a device from the file
systems perspective.  This is fine for xfs and ext4 who's metadata is
being overwritten and already allocated on the thin provisioned device.
They get an EIO on data writes, fstrim to free up the space, and keep it
going.

Btrfs however has dynamic metadata, so the rm -rf could result in
metadata IO being done on the file system.  Since the thin provisioned
device is out of space this gives us an EIO, and we flip read only.  We
didn't remove the file, so the fstrim doesn't recover space anyway, so
we can't even fstrim and remount.

Make this test for ext4/xfs only, it just simply won't work right for
btrfs in it's current form.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 tests/generic/500 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/generic/500 b/tests/generic/500
index 201d8b9f..1cbd9d65 100755
--- a/tests/generic/500
+++ b/tests/generic/500
@@ -44,7 +44,7 @@ _cleanup()
 rm -f $seqres.full
 
 # real QA test starts here
-_supported_fs generic
+_supported_fs xfs ext4
 _supported_os Linux
 _require_scratch_nocheck
 _require_dm_target thin-pool
-- 
2.21.0

[PATCH 2/2] fstests: move generic/500 -> shared/001

[Josef Bacik]

Now that generic/500 is not for all file systems, move it to shared.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 tests/generic/group                       | 1 -
 tests/{generic/500 => shared/001}         | 0
 tests/{generic/500.out => shared/001.out} | 0
 tests/shared/group                        | 1 +
 4 files changed, 1 insertion(+), 1 deletion(-)
 rename tests/{generic/500 => shared/001} (100%)
 rename tests/{generic/500.out => shared/001.out} (100%)

diff --git a/tests/generic/group b/tests/generic/group
index 2e4a6f79..72241abc 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -502,7 +502,6 @@
 497 auto quick swap collapse
 498 auto quick log
 499 auto quick rw collapse zero
-500 auto thin trim
 501 auto quick clone log
 502 auto quick log
 503 auto quick dax punch collapse zero
diff --git a/tests/generic/500 b/tests/shared/001
similarity index 100%
rename from tests/generic/500
rename to tests/shared/001
diff --git a/tests/generic/500.out b/tests/shared/001.out
similarity index 100%
rename from tests/generic/500.out
rename to tests/shared/001.out
diff --git a/tests/shared/group b/tests/shared/group
index a8b926d8..c081eb93 100644
--- a/tests/shared/group
+++ b/tests/shared/group
@@ -3,6 +3,7 @@
 # - do not start group names with a digit
 # - comment line before each group is "new" description
 #
+001 auto thin trim
 002 auto metadata quick log
 032 mkfs auto quick
 298 auto trim
-- 
2.21.0

Re: [PATCH 1/2] fstests: make generic/500 xfs+ext4 only

[Darrick J. Wong]

On Thu, Aug 15, 2019 at 11:00:31AM -0400, Josef Bacik wrote:
> I recently fixed some bugs in btrfs's enospc handling that made it start
> failing generic/500.
> 
> The point of this test is to make the thin provisioned device run out of
> space, which results in an EIO being seen on a device from the file
> systems perspective.  This is fine for xfs and ext4 who's metadata is
> being overwritten and already allocated on the thin provisioned device.
> They get an EIO on data writes, fstrim to free up the space, and keep it
> going.
> 
> Btrfs however has dynamic metadata, so the rm -rf could result in
> metadata IO being done on the file system.  Since the thin provisioned
> device is out of space this gives us an EIO, and we flip read only.  We
> didn't remove the file, so the fstrim doesn't recover space anyway, so
> we can't even fstrim and remount.
> 
> Make this test for ext4/xfs only, it just simply won't work right for
> btrfs in it's current form.

How about:

test $FSTYP = "btrfs" && _notrun "btrfs doesn't work that way lol"

since afaik btrfs is the only fs that shouldn't run this test?
Also, I think Ted was trying to kill off tests/shared/...

--D

> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>  tests/generic/500 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/generic/500 b/tests/generic/500
> index 201d8b9f..1cbd9d65 100755
> --- a/tests/generic/500
> +++ b/tests/generic/500
> @@ -44,7 +44,7 @@ _cleanup()
>  rm -f $seqres.full
>  
>  # real QA test starts here
> -_supported_fs generic
> +_supported_fs xfs ext4
>  _supported_os Linux
>  _require_scratch_nocheck
>  _require_dm_target thin-pool
> -- 
> 2.21.0
> 

Re: [PATCH 1/2] fstests: make generic/500 xfs+ext4 only

[Josef Bacik]

On Thu, Aug 15, 2019 at 08:24:25AM -0700, Darrick J. Wong wrote:
> On Thu, Aug 15, 2019 at 11:00:31AM -0400, Josef Bacik wrote:
> > I recently fixed some bugs in btrfs's enospc handling that made it start
> > failing generic/500.
> > 
> > The point of this test is to make the thin provisioned device run out of
> > space, which results in an EIO being seen on a device from the file
> > systems perspective.  This is fine for xfs and ext4 who's metadata is
> > being overwritten and already allocated on the thin provisioned device.
> > They get an EIO on data writes, fstrim to free up the space, and keep it
> > going.
> > 
> > Btrfs however has dynamic metadata, so the rm -rf could result in
> > metadata IO being done on the file system.  Since the thin provisioned
> > device is out of space this gives us an EIO, and we flip read only.  We
> > didn't remove the file, so the fstrim doesn't recover space anyway, so
> > we can't even fstrim and remount.
> > 
> > Make this test for ext4/xfs only, it just simply won't work right for
> > btrfs in it's current form.
> 
> How about:
> 
> test $FSTYP = "btrfs" && _notrun "btrfs doesn't work that way lol"
> 
> since afaik btrfs is the only fs that shouldn't run this test?
> Also, I think Ted was trying to kill off tests/shared/...
> 

That explains why it seemed emptier than normal.  Yeah that sounds like a fine
solution to me, I'll send a updated patch shortly.  Thanks,

Josef