Re: [PATCH v2 2/2] fsx: Add '-a' option to skip unsupported keep size automatically

[Amir Goldstein <amir73il@gmail.com>]

On Mon, Jan 6, 2020 at 9:12 AM Xiao Yang <yangx.jy@cn.fujitsu.com> wrote:
>
> 1) Use '-a' option to skip unsupported keep size automatically even if
>    it's defined by --replay-ops.
> 2) Add $FSX_AVOID to tests which defines keep size by --replay-ops.
>
> Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
> ---
>  ltp/fsx.c         | 9 +++++++--
>  tests/generic/456 | 2 +-
>  tests/generic/469 | 2 +-
>  tests/generic/499 | 2 +-
>  tests/generic/511 | 2 +-
>  5 files changed, 11 insertions(+), 6 deletions(-)
>

A very strong NACK!

Why are you making this change?
Did you read the description of the tests and try to understand the sequence
prescribed in the replay-ops file?

Those are reproducers to specific issues that require a very specific sequence
of operations and it seems to me that 'keep_size' is there for a reason in every
one of those tests.

For example, take the test generic/456, which I wrote, it has this link in the
comment above fsxops to a very elaborate email from Ted explaining the
problem: https://marc.info/?l=linux-ext4&m=151137380830381&w=2

You cannot just remove 'keep_size' from the test because then the test
doesn't do what it is intended to do.

Did you read my reply to Eryu's patch which he referred you to?
https://spinics.net/lists/fstests/msg08007.html

Instead of allowing test generic/456 to run on fs which doesn't support
FL_KEEP_SIZE, you should change the test to *require* support for
FL_KEEP_SIZE as well as require support for punch/zero/collapse:

_require_xfs_io_command "falloc" "-k"
_require_xfs_io_command "fpunch"
_require_xfs_io_command "fzero"
_require_xfs_io_command "fcollapse"

Same for the other tests that you changed to ignore keep_size.

Thanks,
Amir.