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

[Zorro Lang <zlang@redhat.com>]

On Sun, Nov 06, 2022 at 04:44:05PM -0500, Theodore Ts'o wrote:
> On Sun, Nov 06, 2022 at 08:10:31PM +0800, Zorro Lang wrote:
> > 
> > Thanks Ted, actually I'm going to merge this patch (with g/457 fix by myself)
> > this week.
> > 
> > But looks like you missed the change on g/457 (might forgot to commit). Anyway,
> > I think it's not worth wasting one more week for this small change, I'll help
> > to change g/457 when I merge this patch.
> 
> Argh, yeah, sorry, I forgot to do the "git add -u".  Anyway, here's
> the leftover change that was in my git working directory, if it saves
> you 5 seconds or so.  :-)

Hi Ted,

Thanks, I've pushed this patch last night :)

> 
> BTW, I noticed there were a number of fsx --replay-ops invocations
> where we could potentially add the $FSX_AVOID.  OTOH, it would
> probably make those tests completely pointless, so it might be easier
> just for the test runners to relay on a group-based exclusion in those
> cases.   What do you think?

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. Likes g/456, it does:

  write 0x137dd 0xdc69 0x0
  fallocate 0xb531 0xb5ad 0x21446
  collapse_range 0x1c000 0x4000 0x21446
  write 0x3e5ec 0x1a14 0x21446
  zero_range 0x20fac 0x6d9c 0x40000 keep_size
  mapwrite 0x216ad 0x274f 0x40000

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 :)

Thanks,
Zorro

> 
> 				- Ted
> 
> diff --git a/tests/generic/457 b/tests/generic/457
> index da75798f1..ca0f5e622 100755
> --- a/tests/generic/457
> +++ b/tests/generic/457
> @@ -83,7 +83,7 @@ FSX_OPTS="-N $NUM_OPS -d -k -P $SANITY_DIR -i $LOGWRITES_DMDEV"
>  for j in `seq 0 $((NUM_FILES-1))`; do
>  	# clone the clone from prev iteration which may have already mutated
>  	_cp_reflink $SCRATCH_MNT/testfile$((j-1)) $SCRATCH_MNT/testfile$j
> -	run_check $here/ltp/fsx $FSX_OPTS -S 0 -j $j $SCRATCH_MNT/testfile$j &
> +	run_check $here/ltp/fsx $FSX_OPTS $FSX_AVOID -S 0 -j $j $SCRATCH_MNT/testfile$j &
>  done
>  wait
>  
>