Re: [PATCH] generic: add missing $FSX_AVOID to fsx invocations

["Darrick J. Wong" <djwong@kernel.org>]

On Mon, Nov 07, 2022 at 11:35:16AM -0500, Theodore Ts'o wrote:
> On Mon, Nov 07, 2022 at 10:02:36AM +0800, Zorro Lang wrote:
> > I think it doesn't make sense to use $FSX_AVOID in `fsx --replay-ops` cases.
> > Due to generally the operations which a cases would like to replay are exact
> > steps to reproduce to a known bug. If we skip some operations (e.g. -F), it
> > doesn't make sense for this reproducer.
> > 
> > The recommended way for this kind of cases is making sure current fs/system
> > support the operations will be run by fsx, especially those features are not
> > common on different fs/system....
> >
> > So it uses below _require_* helpers to make sure these operations are supported,
> > before testing:
> > 
> >   _require_xfs_io_command "falloc"
> >   _require_xfs_io_command "falloc" "-k"
> >   _require_xfs_io_command "fzero"
> >   _require_xfs_io_command "fcollapse"
> > 
> > That's my point, hope I didn't misunderstand what you said :)
> 
> No, you didn't understand me.  :-)
> 
> For context, I have an out of tree patch (see attached), which I had
> tried upstreaming a while back, but it got rejected, so I've continued
> to keep it in my personal tree.  The basic idea is sometimes you might
> want to suppress a test even *though* _require_xfs_io_command seems to
> indicate that operation was supported.
> 
> This might either be because the test didn't know about ext4
> bigalloc's cluster alignment requirements, or because a particular
> operation might just be *buggy* and being able to run tests as if a
> particular command wasn't supported was useful.
> 
> It was rejected because the claim was that you could just exclude by
> group instead (e.g., "punch", "collapse") but I didn't trust that the
> group list would be kept up to date, so I never really agreed with
> that line of reasoning.  These days, given that group declaration are
> kept in the test script, it's much less likely to happen, but I've
> kept the patch in my tree because it's occasionally useful.
> 
> At this point, it's admittedly pretty rarely needed since ext4's
> collapse and insert range commands are pretty solid modulo tests not
> understanding cluster alignment, but still, it's not much effort for
> me to keep carrying the patch and I don't expect it will ever get
> upstreamed.

If it's collapse/insert range you're specifically worried about, perhaps
its time to implement _get_file_block_size for ext4 so that
_test_congruent_file_oplen can exclude those tests that will get the
alignment wrong?

--D

> 
> 					- Ted
> 
> commit c9d25475a94d5e53d7f18d247a17088999522862
> Author: Theodore Ts'o <tytso@mit.edu>
> Date:   Sat Oct 17 14:39:26 2015 -0400
> 
>     common: introduce XFS_IO_AVOID env var
>     
>     Like FSSTRESS_AVOID and FSX_AVOID, XFS_IO_AVOID can be used to avoid
>     using various advanced file system features such as "fpunch"
>     "fcollapse", "finsert", or "zero".  Tests that require an xfs_io
>     command which is included in the space-separated list found in the
>     XFS_IO_AVOID environment variable will be skipped using _notrun.
>     
>     Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> 
> diff --git a/README b/README
> index 4c4f22f85..42baff07b 100644
> --- a/README
> +++ b/README
> @@ -245,6 +245,10 @@ Misc:
>     this option is supported for all filesystems currently only -overlay is
>     expected to run without issues. For other filesystems additional patches
>     and fixes to the test suite might be needed.
> + - setenv XFS_IO_AVOID, which may contain a list of space separated
> +   xfs_io commands which will be avoided in case you want to exclude
> +   tests that require the use of certain file system operations such
> +   as "fpunch", "fcollapse", "finsert", or "zero".
>  
>  ______________________
>  USING THE FSQA SUITE
> diff --git a/common/rc b/common/rc
> index eb67e0cdc..d1c07a4d0 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2485,6 +2485,11 @@ _require_xfs_io_command()
>  	local opts=""
>  	local attr_info=""
>  
> +	if echo "$XFS_IO_AVOID" | grep -wq -- "$command"
> +	then
> +		_notrun "Avoiding xfs_io $command"
> +	fi
> +
>  	local testfile=$TEST_DIR/$$.xfs_io
>  	local testio
>  	case $command in