2022-02-09 09:56:47

by Anna Schumaker

[permalink] [raw]
Subject: [PATCH 3/4] generic/578: Test that filefrag is supported before running

From: Anna Schumaker <[email protected]>

NFS does not support FIBMAP/FIEMAP, so the check for non-shared extents
on NFS v4.2 always fails with the message: "FIBMAP/FIEMAP unsupported".
I added the _require_filefrag check for NFS and other filesystems that
don't have FIEMAP or FIBMAP support.

Signed-off-by: Anna Schumaker <[email protected]>
---
common/rc | 14 ++++++++++++++
tests/generic/578 | 2 +-
2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/common/rc b/common/rc
index b3289de985d8..73d17da9430e 100644
--- a/common/rc
+++ b/common/rc
@@ -4673,6 +4673,20 @@ _require_inode_limits()
fi
}

+_require_filefrag()
+{
+ _require_command "$FILEFRAG_PROG" filefrag
+
+ local file="$TEST_DIR/filefrag_testfile"
+
+ echo "XX" > $file
+ ${FILEFRAG_PROG} $file 2>&1 | grep -q "FIBMAP/FIEMAP[[:space:]]*unsupported"
+ if [ $? -eq 0 ]; then
+ _notrun "FIBMAP/FIEMAP not supported by this filesystem"
+ fi
+ rm -f $file
+}
+
_require_filefrag_options()
{
_require_command "$FILEFRAG_PROG" filefrag
diff --git a/tests/generic/578 b/tests/generic/578
index 01929a280f8c..64c813032cf8 100755
--- a/tests/generic/578
+++ b/tests/generic/578
@@ -23,7 +23,7 @@ _cleanup()
# real QA test starts here
_supported_fs generic
_require_test_program "mmap-write-concurrent"
-_require_command "$FILEFRAG_PROG" filefrag
+_require_filefrag
_require_test_reflink
_require_cp_reflink

--
2.35.1



2022-02-23 17:25:41

by Darrick J. Wong

[permalink] [raw]
Subject: Re: [PATCH 3/4] generic/578: Test that filefrag is supported before running

On Tue, Feb 08, 2022 at 04:52:31PM -0500, Anna Schumaker wrote:
> From: Anna Schumaker <[email protected]>
>
> NFS does not support FIBMAP/FIEMAP, so the check for non-shared extents
> on NFS v4.2 always fails with the message: "FIBMAP/FIEMAP unsupported".
> I added the _require_filefrag check for NFS and other filesystems that
> don't have FIEMAP or FIBMAP support.
>
> Signed-off-by: Anna Schumaker <[email protected]>
> ---
> common/rc | 14 ++++++++++++++
> tests/generic/578 | 2 +-
> 2 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/common/rc b/common/rc
> index b3289de985d8..73d17da9430e 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -4673,6 +4673,20 @@ _require_inode_limits()
> fi
> }
>
> +_require_filefrag()
> +{
> + _require_command "$FILEFRAG_PROG" filefrag
> +
> + local file="$TEST_DIR/filefrag_testfile"
> +
> + echo "XX" > $file

Nit: You might want to rm -f $file before echoing into it, just in case
some future knave ;) sets up that pathname to point to a named pipe or
something that will hang fstests...

> + ${FILEFRAG_PROG} $file 2>&1 | grep -q "FIBMAP/FIEMAP[[:space:]]*unsupported"
> + if [ $? -eq 0 ]; then

...and rm it again here to avoid leaving test files around.

Otherwise this looks ok to me.

--D

> + _notrun "FIBMAP/FIEMAP not supported by this filesystem"
> + fi
> + rm -f $file
> +}
> +
> _require_filefrag_options()
> {
> _require_command "$FILEFRAG_PROG" filefrag
> diff --git a/tests/generic/578 b/tests/generic/578
> index 01929a280f8c..64c813032cf8 100755
> --- a/tests/generic/578
> +++ b/tests/generic/578
> @@ -23,7 +23,7 @@ _cleanup()
> # real QA test starts here
> _supported_fs generic
> _require_test_program "mmap-write-concurrent"
> -_require_command "$FILEFRAG_PROG" filefrag
> +_require_filefrag
> _require_test_reflink
> _require_cp_reflink
>
> --
> 2.35.1
>

2022-02-24 01:07:35

by Anna Schumaker

[permalink] [raw]
Subject: Re: [PATCH 3/4] generic/578: Test that filefrag is supported before running

On Wed, Feb 23, 2022 at 11:57 AM Darrick J. Wong <[email protected]> wrote:
>
> On Tue, Feb 08, 2022 at 04:52:31PM -0500, Anna Schumaker wrote:
> > From: Anna Schumaker <[email protected]>
> >
> > NFS does not support FIBMAP/FIEMAP, so the check for non-shared extents
> > on NFS v4.2 always fails with the message: "FIBMAP/FIEMAP unsupported".
> > I added the _require_filefrag check for NFS and other filesystems that
> > don't have FIEMAP or FIBMAP support.
> >
> > Signed-off-by: Anna Schumaker <[email protected]>
> > ---
> > common/rc | 14 ++++++++++++++
> > tests/generic/578 | 2 +-
> > 2 files changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/common/rc b/common/rc
> > index b3289de985d8..73d17da9430e 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -4673,6 +4673,20 @@ _require_inode_limits()
> > fi
> > }
> >
> > +_require_filefrag()
> > +{
> > + _require_command "$FILEFRAG_PROG" filefrag
> > +
> > + local file="$TEST_DIR/filefrag_testfile"
> > +
> > + echo "XX" > $file
>
> Nit: You might want to rm -f $file before echoing into it, just in case
> some future knave ;) sets up that pathname to point to a named pipe or
> something that will hang fstests...
>
> > + ${FILEFRAG_PROG} $file 2>&1 | grep -q "FIBMAP/FIEMAP[[:space:]]*unsupported"
> > + if [ $? -eq 0 ]; then
>
> ...and rm it again here to avoid leaving test files around.

Sure, I can make that change. Should I update
_require_filefrag_options() and _require_fibmap() to follow the same
pattern while I'm at it? The new function was based off of those.

Anna

>
> Otherwise this looks ok to me.
>
> --D
>
> > + _notrun "FIBMAP/FIEMAP not supported by this filesystem"
> > + fi
> > + rm -f $file
> > +}
> > +
> > _require_filefrag_options()
> > {
> > _require_command "$FILEFRAG_PROG" filefrag
> > diff --git a/tests/generic/578 b/tests/generic/578
> > index 01929a280f8c..64c813032cf8 100755
> > --- a/tests/generic/578
> > +++ b/tests/generic/578
> > @@ -23,7 +23,7 @@ _cleanup()
> > # real QA test starts here
> > _supported_fs generic
> > _require_test_program "mmap-write-concurrent"
> > -_require_command "$FILEFRAG_PROG" filefrag
> > +_require_filefrag
> > _require_test_reflink
> > _require_cp_reflink
> >
> > --
> > 2.35.1
> >

2022-02-24 04:42:41

by Darrick J. Wong

[permalink] [raw]
Subject: Re: [PATCH 3/4] generic/578: Test that filefrag is supported before running

On Wed, Feb 23, 2022 at 01:24:08PM -0500, Anna Schumaker wrote:
> On Wed, Feb 23, 2022 at 11:57 AM Darrick J. Wong <[email protected]> wrote:
> >
> > On Tue, Feb 08, 2022 at 04:52:31PM -0500, Anna Schumaker wrote:
> > > From: Anna Schumaker <[email protected]>
> > >
> > > NFS does not support FIBMAP/FIEMAP, so the check for non-shared extents
> > > on NFS v4.2 always fails with the message: "FIBMAP/FIEMAP unsupported".
> > > I added the _require_filefrag check for NFS and other filesystems that
> > > don't have FIEMAP or FIBMAP support.
> > >
> > > Signed-off-by: Anna Schumaker <[email protected]>
> > > ---
> > > common/rc | 14 ++++++++++++++
> > > tests/generic/578 | 2 +-
> > > 2 files changed, 15 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/common/rc b/common/rc
> > > index b3289de985d8..73d17da9430e 100644
> > > --- a/common/rc
> > > +++ b/common/rc
> > > @@ -4673,6 +4673,20 @@ _require_inode_limits()
> > > fi
> > > }
> > >
> > > +_require_filefrag()
> > > +{
> > > + _require_command "$FILEFRAG_PROG" filefrag
> > > +
> > > + local file="$TEST_DIR/filefrag_testfile"
> > > +
> > > + echo "XX" > $file
> >
> > Nit: You might want to rm -f $file before echoing into it, just in case
> > some future knave ;) sets up that pathname to point to a named pipe or
> > something that will hang fstests...
> >
> > > + ${FILEFRAG_PROG} $file 2>&1 | grep -q "FIBMAP/FIEMAP[[:space:]]*unsupported"
> > > + if [ $? -eq 0 ]; then
> >
> > ...and rm it again here to avoid leaving test files around.
>
> Sure, I can make that change. Should I update
> _require_filefrag_options() and _require_fibmap() to follow the same
> pattern while I'm at it? The new function was based off of those.

Yes please, and thank you! :)

--D

> Anna
>
> >
> > Otherwise this looks ok to me.
> >
> > --D
> >
> > > + _notrun "FIBMAP/FIEMAP not supported by this filesystem"
> > > + fi
> > > + rm -f $file
> > > +}
> > > +
> > > _require_filefrag_options()
> > > {
> > > _require_command "$FILEFRAG_PROG" filefrag
> > > diff --git a/tests/generic/578 b/tests/generic/578
> > > index 01929a280f8c..64c813032cf8 100755
> > > --- a/tests/generic/578
> > > +++ b/tests/generic/578
> > > @@ -23,7 +23,7 @@ _cleanup()
> > > # real QA test starts here
> > > _supported_fs generic
> > > _require_test_program "mmap-write-concurrent"
> > > -_require_command "$FILEFRAG_PROG" filefrag
> > > +_require_filefrag
> > > _require_test_reflink
> > > _require_cp_reflink
> > >
> > > --
> > > 2.35.1
> > >