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

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

[Cyril Hrubis]

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)
+{
+	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
 }
 
-- 
2.49.0


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

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

[Petr Vorel]

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

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

[Li Wang via ltp]

Petr Vorel <pvorel@suse.cz> wrote:

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

Yeah, looks good.

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

Reviewed-by: Li Wang <liwang@redhat.com>

-- 
Regards,
Li Wang


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

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

[Li Wang via ltp]

Petr Vorel <pvorel@suse.cz> wrote:

> > -     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;

One more thing I'm a bit hesitant about, do we need to explicitly set
fs_types[1] to NULL here?

> >               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;

fs_type[1] = NULL; ?

-- 
Regards,
Li Wang


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

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

[Cyril Hrubis]

Hi!
> One more thing I'm a bit hesitant about, do we need to explicitly set
> fs_types[1] to NULL here?

It's a static variable so the whole array is initialized to NULL by the
compiler.

-- 
Cyril Hrubis
chrubis@suse.cz

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

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

[Petr Vorel]

Hi Li, Cyril,

> Petr Vorel <pvorel@suse.cz> wrote:

> > > -     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;

> One more thing I'm a bit hesitant about, do we need to explicitly set
> fs_types[1] to NULL here?

Although it looks reasonable, I suppose it's not needed because array is static,
therefore members are NULL, right?

static const char *fs_types[ARRAY_SIZE(fs_type_whitelist)];

Also I tested it on statmount02.c, which has:

	.all_filesystems = 1,
	.skip_filesystems = (const char *const []) {
		"fuse",
		"btrfs",
		NULL
	},

and it works as expected:

# LTP_SINGLE_FS_TYPE=ntfs ./statmount02
...
tst_test.c:1953: TINFO: LTP version: 20250130-278-gada0e52d26
...
tst_supported_fs_types.c:190: TINFO: WARNING: testing only ntfs
tst_supported_fs_types.c:132: TINFO: FUSE does support ntfs
tst_supported_fs_types.c:62: TINFO: mkfs.ntfs does exist
tst_supported_fs_types.c:163: TINFO: Skipping FUSE based ntfs as requested by the test
tst_test.c:1917: TCONF: There are no supported filesystems

# LTP_FORCE_SINGLE_FS_TYPE=ntfs ./statmount02
...
tst_test.c:1953: TINFO: LTP version: 20250130-278-gada0e52d26
...
tst_supported_fs_types.c:197: TINFO: WARNING: force testing only ntfs
tst_test.c:1888: TINFO: === Testing on ntfs ===
tst_test.c:1217: TINFO: Formatting /dev/loop0 with ntfs opts='' extra opts=''
The partition start sector was not specified for /dev/loop0 and it could not be obtained automatically.  It has been set to 0.
The number of sectors per track was not specified for /dev/loop0 and it could not be obtained automatically.  It has been set to 0.
The number of heads was not specified for /dev/loop0 and it could not be obtained automatically.  It has been set to 0.
To boot from a device, Windows needs the 'partition start sector', the 'sectors per track' and the 'number of heads' to be set.
Windows will not be able to boot from this device.
statmount02.c:51: TINFO: Mounting /dev/loop0 to /tmp/LTP_sta2GSo2A/mntpoint fstyp=ntfs flags=1
statmount02.c:51: TINFO: Trying FUSE...
statmount02.c:35: TPASS: statmount(sx_mount->data.stx_mnt_id, STATMOUNT_SB_BASIC, st_mount, sizeof(struct statmount), 0) passed
statmount02.c:41: TPASS: st_mount->mask == STATMOUNT_SB_BASIC (1)
statmount02.c:42: TPASS: st_mount->size == sizeof(struct statmount) (512)
statmount02.c:43: TPASS: st_mount->sb_dev_major == sx_mount->data.stx_dev_major (7)
statmount02.c:44: TPASS: st_mount->sb_dev_minor == sx_mount->data.stx_dev_minor (0)
statmount02.c:45: TPASS: st_mount->sb_magic == sf_mount->f_type (1702057286)
statmount02.c:46: TFAIL: st_mount->sb_flags (0) != MS_RDONLY (1)
statmount02.c:60: TINFO: Umounting /tmp/LTP_sta2GSo2A/mntpoint

Kind regards,
Petr

> > >               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;

> fs_type[1] = NULL; ?

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

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

[Li Wang via ltp]

On Tue, May 27, 2025 at 5:31 PM Cyril Hrubis <chrubis@suse.cz> wrote:
>
> Hi!
> > One more thing I'm a bit hesitant about, do we need to explicitly set
> > fs_types[1] to NULL here?
>
> It's a static variable so the whole array is initialized to NULL by the
> compiler.

True, unless the function  tst_get_supported_fs_types() is being called
many times (which could change the fs_types contents), but so far I
find it only been used once in tst_test.c.

It should be fine by now.

--
Regards,
Li Wang


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

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

[Cyril Hrubis]

Hi!
> > > One more thing I'm a bit hesitant about, do we need to explicitly set
> > > fs_types[1] to NULL here?
> >
> > It's a static variable so the whole array is initialized to NULL by the
> > compiler.
> 
> True, unless the function  tst_get_supported_fs_types() is being called
> many times (which could change the fs_types contents), but so far I
> find it only been used once in tst_test.c.

Indeed, it's a function that's used only from within the tst_test.c and
not supposed to be used in tests.

-- 
Cyril Hrubis
chrubis@suse.cz

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

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

[Li Wang via ltp]

Petr Vorel <pvorel@suse.cz> wrote:

>
> > One more thing I'm a bit hesitant about, do we need to explicitly set
> > fs_types[1] to NULL here?
>
> Although it looks reasonable, I suppose it's not needed because array is static,
> therefore members are NULL, right?

That's right, just confirmed with Cyril, so far we need to do nothing
unless tst_get_supported_fs_types() is abused somewhere.

Thanks for the quick reply, and feel free to merge this one.

-- 
Regards,
Li Wang


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

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

[Petr Vorel]

> Petr Vorel <pvorel@suse.cz> wrote:


> > > One more thing I'm a bit hesitant about, do we need to explicitly set
> > > fs_types[1] to NULL here?

> > Although it looks reasonable, I suppose it's not needed because array is static,
> > therefore members are NULL, right?

> That's right, just confirmed with Cyril, so far we need to do nothing
> unless tst_get_supported_fs_types() is abused somewhere.

Abusing the function should be prevented if Cyril adds before merge 'static'
keyword to the function (as I suggested earlier).

Kind regards,
Petr

> Thanks for the quick reply, and feel free to merge this one.

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

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

[Cyril Hrubis]

Hi!
> > That's right, just confirmed with Cyril, so far we need to do nothing
> > unless tst_get_supported_fs_types() is abused somewhere.
> 
> Abusing the function should be prevented if Cyril adds before merge 'static'
> keyword to the function (as I suggested earlier).

That's a different function and that is going to be static in the final
patch.

This is the function that is called from tst_test.c and I guess that we
can remove it from the tst_fs.h in order to make sure nobody tries to
misuse it. That would look like:

diff --git a/include/tst_fs.h b/include/tst_fs.h
index 19c240e07..ceae78e7e 100644
--- a/include/tst_fs.h
+++ b/include/tst_fs.h
@@ -203,13 +203,6 @@ enum tst_fs_impl {
  */
 enum tst_fs_impl tst_fs_is_supported(const char *fs_type);

-/*
- * Returns NULL-terminated array of kernel-supported filesystems.
- *
- * @skiplist A NULL terminated array of filesystems to skip.
- */
-const char **tst_get_supported_fs_types(const char *const *skiplist);
-
 /*
  * Returns 1 if filesystem is in skiplist 0 otherwise.
  *
diff --git a/lib/tst_internal.h b/lib/tst_internal.h
new file mode 100644
index 000000000..a0ae733f6
--- /dev/null
+++ b/lib/tst_internal.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (c) 2015-2025 Cyril Hrubis <chrubis@suse.cz>
+ * Copyright (c) Linux Test Project, 2017-2025
+ */
+
+#ifndef TST_INTERNAL_H__
+#define TST_INTERNAL_H__
+
+/*
+ * Returns NULL-terminated array of kernel-supported filesystems.
+ *
+ * @skiplist A NULL terminated array of filesystems to skip.
+ */
+const char **tst_get_supported_fs_types(const char *const *skiplist);
+
+#endif /* TST_INTERNAL_H__ */
diff --git a/lib/tst_supported_fs_types.c b/lib/tst_supported_fs_types.c
index 46165ebc6..cc5522a08 100644
--- a/lib/tst_supported_fs_types.c
+++ b/lib/tst_supported_fs_types.c
@@ -14,6 +14,7 @@
 #define TST_NO_DEFAULT_MAIN
 #include "tst_test.h"
 #include "tst_fs.h"
+#include "tst_internal.h"

 /*
  * NOTE: new filesystem should be also added to
diff --git a/lib/tst_test.c b/lib/tst_test.c
index 45fc28498..f62cc195a 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -38,6 +38,7 @@
 #include "old_device.h"
 #include "old_tmpdir.h"
 #include "ltp-version.h"
+#include "tst_internal.h"

 /*
  * Hack to get TCID defined in newlib tests


-- 
Cyril Hrubis
chrubis@suse.cz

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

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

[Petr Vorel]

Hi Cyril, all,

> Hi!
> > > That's right, just confirmed with Cyril, so far we need to do nothing
> > > unless tst_get_supported_fs_types() is abused somewhere.

> > Abusing the function should be prevented if Cyril adds before merge 'static'
> > keyword to the function (as I suggested earlier).

> That's a different function and that is going to be static in the final
> patch.

Ah, thanks for info.

> This is the function that is called from tst_test.c and I guess that we
> can remove it from the tst_fs.h in order to make sure nobody tries to
> misuse it. That would look like:

> diff --git a/include/tst_fs.h b/include/tst_fs.h
> index 19c240e07..ceae78e7e 100644
> --- a/include/tst_fs.h
> +++ b/include/tst_fs.h
> @@ -203,13 +203,6 @@ enum tst_fs_impl {
>   */
>  enum tst_fs_impl tst_fs_is_supported(const char *fs_type);

> -/*
> - * Returns NULL-terminated array of kernel-supported filesystems.
> - *
> - * @skiplist A NULL terminated array of filesystems to skip.
> - */
> -const char **tst_get_supported_fs_types(const char *const *skiplist);
> -
>  /*
>   * Returns 1 if filesystem is in skiplist 0 otherwise.
>   *
> diff --git a/lib/tst_internal.h b/lib/tst_internal.h
> new file mode 100644
> index 000000000..a0ae733f6
> --- /dev/null
> +++ b/lib/tst_internal.h
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (c) 2015-2025 Cyril Hrubis <chrubis@suse.cz>
> + * Copyright (c) Linux Test Project, 2017-2025
> + */
> +
> +#ifndef TST_INTERNAL_H__
> +#define TST_INTERNAL_H__
> +
> +/*
> + * Returns NULL-terminated array of kernel-supported filesystems.
> + *
> + * @skiplist A NULL terminated array of filesystems to skip.
> + */
> +const char **tst_get_supported_fs_types(const char *const *skiplist);
> +
> +#endif /* TST_INTERNAL_H__ */
> diff --git a/lib/tst_supported_fs_types.c b/lib/tst_supported_fs_types.c
> index 46165ebc6..cc5522a08 100644
> --- a/lib/tst_supported_fs_types.c
> +++ b/lib/tst_supported_fs_types.c
> @@ -14,6 +14,7 @@
>  #define TST_NO_DEFAULT_MAIN
>  #include "tst_test.h"
>  #include "tst_fs.h"
> +#include "tst_internal.h"

>  /*
>   * NOTE: new filesystem should be also added to
> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index 45fc28498..f62cc195a 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -38,6 +38,7 @@
>  #include "old_device.h"
>  #include "old_tmpdir.h"
>  #include "ltp-version.h"
> +#include "tst_internal.h"

>  /*
>   * Hack to get TCID defined in newlib tests

FYI also testcases/lib/tst_supported_fs.c uses it:

tst_supported_fs.c: In function ‘main’:
tst_supported_fs.c:152:23: error: implicit declaration of function ‘tst_get_supported_fs_types’ [-Wimplicit-function-declaration]
  152 |         filesystems = tst_get_supported_fs_types((const char * const*)skiplist);
      |                       ^~~~~~~~~~~~~~~~~~~~~~~~~~
tst_supported_fs.c:152:21: error: assignment to ‘const char * const*’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion]
  152 |         filesystems = tst_get_supported_fs_types((const char * const*)skiplist);
      |                     ^
make: *** [../../include/mk/rules.mk:48: tst_supported_fs] Error 1

Therefore we need either relative path (below) or tweak CFLAGS to add -I.

Otherwise LGTM, ideally send a patch.

Kind regards,
Petr

+++ testcases/lib/tst_supported_fs.c
@@ -14,6 +14,7 @@
 #define TST_NO_DEFAULT_MAIN
 #include "tst_test.h"
 #include "tst_fs.h"
+#include "../../lib/tst_internal.h"

 #define err_exit(...) ({ \
        fprintf(stderr, __VA_ARGS__); \


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

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

[Cyril Hrubis]

Hi!
> FYI also testcases/lib/tst_supported_fs.c uses it:
> 
> tst_supported_fs.c: In function ‘main’:
> tst_supported_fs.c:152:23: error: implicit declaration of function ‘tst_get_supported_fs_types’ [-Wimplicit-function-declaration]
>   152 |         filesystems = tst_get_supported_fs_types((const char * const*)skiplist);
>       |                       ^~~~~~~~~~~~~~~~~~~~~~~~~~
> tst_supported_fs.c:152:21: error: assignment to ‘const char * const*’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion]
>   152 |         filesystems = tst_get_supported_fs_types((const char * const*)skiplist);
>       |                     ^
> make: *** [../../include/mk/rules.mk:48: tst_supported_fs] Error 1
> 
> Therefore we need either relative path (below) or tweak CFLAGS to add -I.

If it's used from shell helper as well we have a tst_private.h header in
include/ for that already, so I guess we should use that one instead.

-- 
Cyril Hrubis
chrubis@suse.cz

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

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

[Petr Vorel]

> Hi!
> > FYI also testcases/lib/tst_supported_fs.c uses it:

> > tst_supported_fs.c: In function ‘main’:
> > tst_supported_fs.c:152:23: error: implicit declaration of function ‘tst_get_supported_fs_types’ [-Wimplicit-function-declaration]
> >   152 |         filesystems = tst_get_supported_fs_types((const char * const*)skiplist);
> >       |                       ^~~~~~~~~~~~~~~~~~~~~~~~~~
> > tst_supported_fs.c:152:21: error: assignment to ‘const char * const*’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion]
> >   152 |         filesystems = tst_get_supported_fs_types((const char * const*)skiplist);
> >       |                     ^
> > make: *** [../../include/mk/rules.mk:48: tst_supported_fs] Error 1

> > Therefore we need either relative path (below) or tweak CFLAGS to add -I.

> If it's used from shell helper as well we have a tst_private.h header in
> include/ for that already, so I guess we should use that one instead.

+1 (I was searched for some specific header, but found only "public"
include/tst_common.h).

Kind regards,
Petr

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

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

[Cyril Hrubis]

Hi!
Thanks for the reviews, pushed.

-- 
Cyril Hrubis
chrubis@suse.cz

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