[1/2] fstests: make generic/500 xfs+ext4 only
diff mbox series

Message ID 20190815150033.15996-1-josef@toxicpanda.com
State New
Headers show
Series
  • [1/2] fstests: make generic/500 xfs+ext4 only
Related show

Commit Message

Josef Bacik Aug. 15, 2019, 3 p.m. UTC
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(-)

Comments

Darrick J. Wong Aug. 15, 2019, 3:24 p.m. UTC | #1
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
>
Josef Bacik Aug. 15, 2019, 3:37 p.m. UTC | #2
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

Patch
diff mbox series

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