[LTP] [PATCH] fanotify22: Make tests not depend on behavior of shutdown filesystem

[Amir Goldstein]

On Fri, Aug 25, 2023 at 3:28 PM Jan Kara <jack@suse.cz> wrote:
>
> The tests in fanotify22 implicitely depended on the fact that filesystem
> shutdown with 'abort' mount option keeps reporting further errors and
> further mounts with 'abort' option. This is however too strict (mostly a
> bug in ext4 implementation) and in principle reporting errors after the
> filesystem is shutdown is just a pointless noise. Ext4 recently modified
> the behavior of 'abort' mount option to behave the same as filesystem
> shutdown and thus also stop reporting further filesystem errors. Modify
> the tests to unmount and mount the filesystem after each test to get it
> out of the shutdown state for the following tests and also replace a
> test testing behavior after mounting with 'abort' mount option with a
> test testing two different filesystem corruption errors.
>
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  .../kernel/syscalls/fanotify/fanotify22.c     | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify22.c b/testcases/kernel/syscalls/fanotify/fanotify22.c
> index 1105172bb269..475155b9f58a 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify22.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify22.c
> @@ -42,6 +42,7 @@
>  #define MOUNT_PATH "test_mnt"
>  #define BASE_DIR "internal_dir"
>  #define BAD_DIR BASE_DIR"/bad_dir"
> +#define BAD_LINK BASE_DIR"/bad_link"
>
>  #ifdef HAVE_NAME_TO_HANDLE_AT
>
> @@ -51,6 +52,7 @@ static int fd_notify;
>  /* These expected FIDs are common to multiple tests */
>  static struct fanotify_fid_t null_fid;
>  static struct fanotify_fid_t bad_file_fid;
> +static struct fanotify_fid_t bad_link_fid;
>
>  static void trigger_fs_abort(void)
>  {
> @@ -78,7 +80,13 @@ static void tcase2_trigger_lookup(void)
>
>  static void tcase3_trigger(void)
>  {
> -       trigger_fs_abort();
> +       int ret;
> +
> +       /* SAFE_OPEN cannot be used here because we expect it to fail. */
> +       ret = open(MOUNT_PATH"/"BAD_LINK, O_RDONLY, 0);
> +       if (ret != -1 && errno != EUCLEAN)
> +               tst_res(TFAIL, "Unexpected open result(%d) of %s (%d!=%d)",
> +                       ret, BAD_LINK, errno, EUCLEAN);
>         tcase2_trigger_lookup();
>  }

To make it more clear that this is a multiple error trigger, I would consider

1. use helper trigger_bad_link_lookup()
2. s/tcase2_trigger_lookup/trigger_bad_file_lookup

AND

static void tcase3_trigger(void)
{
       trigger_bad_link_lookup();
       trigger_bad_file_lookup();
}

With that nit fix, you may add:

Reviewed-by: Amir Goldstein <amir73il@gmail.com>

>
> @@ -113,8 +121,8 @@ static struct test_case {
>                 .name = "Multiple error submission",
>                 .trigger_error = &tcase3_trigger,
>                 .error_count = 2,
> -               .error = ESHUTDOWN,
> -               .fid = &null_fid,
> +               .error = EFSCORRUPTED,
> +               .fid = &bad_link_fid,
>         },
>         {
>                 .name = "Multiple error submission 2",
> @@ -248,6 +256,9 @@ static void do_test(unsigned int i)
>                            FAN_FS_ERROR, AT_FDCWD, MOUNT_PATH);
>
>         check_event(event_buf, read_len, tcase);
> +       /* Unmount and mount the filesystem to get it out of the error state */
> +       SAFE_UMOUNT(MOUNT_PATH);
> +       SAFE_MOUNT(tst_device->dev, MOUNT_PATH, tst_device->fs_type, 0, NULL);
>  }
>
>  static void pre_corrupt_fs(void)
> @@ -256,9 +267,11 @@ static void pre_corrupt_fs(void)
>         SAFE_MKDIR(MOUNT_PATH"/"BAD_DIR, 0777);
>
>         fanotify_save_fid(MOUNT_PATH"/"BAD_DIR, &bad_file_fid);
> +       fanotify_save_fid(MOUNT_PATH"/"BASE_DIR, &bad_link_fid);
>
>         SAFE_UMOUNT(MOUNT_PATH);
>         do_debugfs_request(tst_device->dev, "sif " BAD_DIR " mode 0xff");
> +       do_debugfs_request(tst_device->dev, "ln <1> " BAD_LINK);
>         SAFE_MOUNT(tst_device->dev, MOUNT_PATH, tst_device->fs_type, 0, NULL);
>  }
>
> --
> 2.35.3
>