Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 23 Feb 2022 08:57:29 -0800
From:   "Darrick J. Wong" <djwong@kernel.org>
To:     Anna Schumaker <anna@kernel.org>
Cc:     fstests@vger.kernel.org, linux-nfs@vger.kernel.org
Subject: Re: [PATCH 3/4] generic/578: Test that filefrag is supported before
 running
Message-ID: <20220223165729.GH8288@magnolia>
References: <20220208215232.491780-1-anna@kernel.org>
 <20220208215232.491780-4-anna@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220208215232.491780-4-anna@kernel.org>
Precedence: bulk

On Tue, Feb 08, 2022 at 04:52:31PM -0500, Anna Schumaker wrote:
> From: Anna Schumaker <Anna.Schumaker@Netapp.com>
> 
> 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 <Anna.Schumaker@Netapp.com>
> ---
>  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
>