Re: [LTP] [PATCH v4] lib: LTP_SINGLE_FS_TYPE and LTP_FORCE_SINGLE_FS_TYPE

[Petr Vorel <pvorel@suse.cz>]

Hi Cyril,

@Li @Jan Could you please have a look?
It'd be nice to include it in the release.

> Make the LTP_SINGE_FS_TYPE to use the test skiplists both for
> filesystems and fuse. This fixes the usecase where LTP users want to
> limit the tests with '.all_filesystems' to a single filesystem type
> for a testrun.

> The LTP_FORCE_SINGLE_FS_TYPE now replaces what previously
> LTP_SINGLE_FS_TYPE did and can be used for testing and for that purpose
> it ignores the test skiplists.

> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> Suggested-by:  Petr Vorel <pvorel@suse.cz>
> CC: Jan Polensky <japo@linux.ibm.com>
> ---
>  doc/users/setup_tests.rst    |  5 ++-
>  lib/tst_supported_fs_types.c | 60 ++++++++++++++++++++++++------------
>  lib/tst_test.c               | 25 +++++++--------
>  testcases/lib/tst_test.sh    | 19 ++++++------
>  4 files changed, 67 insertions(+), 42 deletions(-)

> diff --git a/doc/users/setup_tests.rst b/doc/users/setup_tests.rst
> index 2cce85fdf..38976f3b0 100644
> --- a/doc/users/setup_tests.rst
> +++ b/doc/users/setup_tests.rst
> @@ -47,9 +47,12 @@ users.
>         printed by the test (suitable for a reproducible output).

>     * - LTP_SINGLE_FS_TYPE
> -     - Testing only - specifies filesystem instead all supported
> +     - Specifies single filesystem to run the test on instead all supported
>         (for tests with ``.all_filesystems``).

> +   * - LTP_FORCE_SINGLE_FS_TYPE
> +     - Testing only. Behaves like LTP_SINGLE_FS_TYPE but ignores test skiplists.
> +
>     * - LTP_DEV_FS_TYPE
>       - Filesystem used for testing (default: ``ext2``).

> diff --git a/lib/tst_supported_fs_types.c b/lib/tst_supported_fs_types.c
> index bbbb8df19..5e9be1eda 100644
> --- a/lib/tst_supported_fs_types.c
> +++ b/lib/tst_supported_fs_types.c
> @@ -147,40 +147,60 @@ enum tst_fs_impl tst_fs_is_supported(const char *fs_type)
>  	return TST_FS_UNSUPPORTED;
>  }

> +int fs_could_be_used(const char *fs_type, const char *const *skiplist, int skip_fuse)
This should be also static. Please fix it before merge.
The rest LGTM, thanks for implementing it.

Reviewed-by: Petr Vorel <pvorel@suse.cz>
Tested-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

> +{
> +	enum tst_fs_impl sup;
> +
> +	if (tst_fs_in_skiplist(fs_type, skiplist)) {
> +		tst_res(TINFO, "Skipping %s as requested by the test",
> +			fs_type);
> +		return 0;
> +	}
> +
> +	sup = tst_fs_is_supported(fs_type);
> +
> +	if (skip_fuse && sup == TST_FS_FUSE) {
> +		tst_res(TINFO,
> +			"Skipping FUSE based %s as requested by the test",
> +			fs_type);
> +		return 0;
> +	}
> +
> +	return sup != TST_FS_UNSUPPORTED;
> +}
> +
>  const char **tst_get_supported_fs_types(const char *const *skiplist)
>  {
>  	unsigned int i, j = 0;
>  	int skip_fuse;
> -	enum tst_fs_impl sup;
> -	const char *only_fs;
> +	const char *only_fs, *force_only_fs;

> -	skip_fuse = tst_fs_in_skiplist("fuse", skiplist);
>  	only_fs = getenv("LTP_SINGLE_FS_TYPE");
> +	force_only_fs = getenv("LTP_FORCE_SINGLE_FS_TYPE");
> +
> +	if (only_fs && force_only_fs) {
> +		tst_brk(TBROK,
> +			"Only one of LTP_SINGLE_FS_TYPE and LTP_FORCE_SINGLE_FS_TYPE can be set");
> +		return NULL;
> +	}
> +
> +	skip_fuse = tst_fs_in_skiplist("fuse", skiplist);

>  	if (only_fs) {
>  		tst_res(TINFO, "WARNING: testing only %s", only_fs);
> -		if (tst_fs_is_supported(only_fs))
> +		if (fs_could_be_used(only_fs, skiplist, skip_fuse))
>  			fs_types[0] = only_fs;
>  		return fs_types;
>  	}

> -	for (i = 0; fs_type_whitelist[i]; i++) {
> -		if (tst_fs_in_skiplist(fs_type_whitelist[i], skiplist)) {
> -			tst_res(TINFO, "Skipping %s as requested by the test",
> -				fs_type_whitelist[i]);
> -			continue;
> -		}
> -
> -		sup = tst_fs_is_supported(fs_type_whitelist[i]);
> -
> -		if (skip_fuse && sup == TST_FS_FUSE) {
> -			tst_res(TINFO,
> -				"Skipping FUSE based %s as requested by the test",
> -				fs_type_whitelist[i]);
> -			continue;
> -		}
> +	if (force_only_fs) {
> +		tst_res(TINFO, "WARNING: force testing only %s", force_only_fs);
> +		fs_types[0] = force_only_fs;
> +		return fs_types;
> +	}

> -		if (sup)
> +	for (i = 0; fs_type_whitelist[i]; i++) {
> +		if (fs_could_be_used(fs_type_whitelist[i], skiplist, skip_fuse))
>  			fs_types[j++] = fs_type_whitelist[i];
>  	}

> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index d1268535c..45fc28498 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -611,18 +611,19 @@ static void print_help(void)
>  	/* see doc/users/setup_tests.rst, which lists also shell API variables */
>  	fprintf(stderr, "Environment Variables\n");
>  	fprintf(stderr, "---------------------\n");
> -	fprintf(stderr, "KCONFIG_PATH            Specify kernel config file\n");
> -	fprintf(stderr, "KCONFIG_SKIP_CHECK      Skip kernel config check if variable set (not set by default)\n");
> -	fprintf(stderr, "LTPROOT                 Prefix for installed LTP (default: /opt/ltp)\n");
> -	fprintf(stderr, "LTP_COLORIZE_OUTPUT     Force colorized output behaviour (y/1 always, n/0: never)\n");
> -	fprintf(stderr, "LTP_DEV                 Path to the block device to be used (for .needs_device)\n");
> -	fprintf(stderr, "LTP_DEV_FS_TYPE         Filesystem used for testing (default: %s)\n", DEFAULT_FS_TYPE);
> -	fprintf(stderr, "LTP_REPRODUCIBLE_OUTPUT Values 1 or y discard the actual content of the messages printed by the test\n");
> -	fprintf(stderr, "LTP_SINGLE_FS_TYPE      Testing only - specifies filesystem instead all supported (for .all_filesystems)\n");
> -	fprintf(stderr, "LTP_TIMEOUT_MUL         Timeout multiplier (must be a number >=1)\n");
> -	fprintf(stderr, "LTP_RUNTIME_MUL         Runtime multiplier (must be a number >=1)\n");
> -	fprintf(stderr, "LTP_VIRT_OVERRIDE       Overrides virtual machine detection (values: \"\"|kvm|microsoft|xen|zvm)\n");
> -	fprintf(stderr, "TMPDIR                  Base directory for template directory (for .needs_tmpdir, default: %s)\n", TEMPDIR);
> +	fprintf(stderr, "KCONFIG_PATH             Specify kernel config file\n");
> +	fprintf(stderr, "KCONFIG_SKIP_CHECK       Skip kernel config check if variable set (not set by default)\n");
> +	fprintf(stderr, "LTPROOT                  Prefix for installed LTP (default: /opt/ltp)\n");
> +	fprintf(stderr, "LTP_COLORIZE_OUTPUT      Force colorized output behaviour (y/1 always, n/0: never)\n");
> +	fprintf(stderr, "LTP_DEV                  Path to the block device to be used (for .needs_device)\n");
> +	fprintf(stderr, "LTP_DEV_FS_TYPE          Filesystem used for testing (default: %s)\n", DEFAULT_FS_TYPE);
> +	fprintf(stderr, "LTP_REPRODUCIBLE_OUTPUT  Values 1 or y discard the actual content of the messages printed by the test\n");
> +	fprintf(stderr, "LTP_SINGLE_FS_TYPE       Specifies filesystem instead all supported (for .all_filesystems)\n");
> +	fprintf(stderr, "LTP_FORCE_SINGLE_FS_TYPE Testing only. The same as LTP_SINGLE_FS_TYPE but ignores test skiplist.\n");
> +	fprintf(stderr, "LTP_TIMEOUT_MUL          Timeout multiplier (must be a number >=1)\n");
> +	fprintf(stderr, "LTP_RUNTIME_MUL          Runtime multiplier (must be a number >=1)\n");
> +	fprintf(stderr, "LTP_VIRT_OVERRIDE        Overrides virtual machine detection (values: \"\"|kvm|microsoft|xen|zvm)\n");
> +	fprintf(stderr, "TMPDIR                   Base directory for template directory (for .needs_tmpdir, default: %s)\n", TEMPDIR);
>  	fprintf(stderr, "\n");

>  	fprintf(stderr, "Timeout and runtime\n");
> diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
> index 50269d40f..c32bd8b19 100644
> --- a/testcases/lib/tst_test.sh
> +++ b/testcases/lib/tst_test.sh
> @@ -482,15 +482,16 @@ tst_usage()

>  Environment Variables
>  ---------------------
> -KCONFIG_PATH         Specify kernel config file
> -KCONFIG_SKIP_CHECK   Skip kernel config check if variable set (not set by default)
> -LTPROOT              Prefix for installed LTP (default: /opt/ltp)
> -LTP_COLORIZE_OUTPUT  Force colorized output behaviour (y/1 always, n/0: never)
> -LTP_DEV              Path to the block device to be used (for .needs_device)
> -LTP_DEV_FS_TYPE      Filesystem used for testing (default: ext2)
> -LTP_SINGLE_FS_TYPE   Testing only - specifies filesystem instead all supported (for TST_ALL_FILESYSTEMS=1)
> -LTP_TIMEOUT_MUL      Timeout multiplier (must be a number >=1, ceiled to int)
> -TMPDIR               Base directory for template directory (for .needs_tmpdir, default: /tmp)
> +KCONFIG_PATH             Specify kernel config file
> +KCONFIG_SKIP_CHECK       Skip kernel config check if variable set (not set by default)
> +LTPROOT                  Prefix for installed LTP (default: /opt/ltp)
> +LTP_COLORIZE_OUTPUT      Force colorized output behaviour (y/1 always, n/0: never)
> +LTP_DEV                  Path to the block device to be used (for .needs_device)
> +LTP_DEV_FS_TYPE          Filesystem used for testing (default: ext2)
> +LTP_SINGLE_FS_TYPE       Specifies filesystem instead all supported (for TST_ALL_FILESYSTEMS=1)
> +LTP_FORCE_SINGLE_FS_TYPE Testing only. The same as LTP_SINGLE_FS_TYPE but ignores test skiplist
> +LTP_TIMEOUT_MUL          Timeout multiplier (must be a number >=1, ceiled to int)
> +TMPDIR                   Base directory for template directory (for .needs_tmpdir, default: /tmp)
>  EOF
>  }

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp