[LTP] [PATCH v2 0/6] Add TST_FORMAT_DEVICE support

[LTP] [PATCH v2 0/6] Add TST_FORMAT_DEVICE support

[Petr Vorel]

Hi,

changes v1->v2:
* Replace $TST_DEFAULT_FS_TYPE with proper $TST_FS_TYPE initialization
  (Cyril)
* Redesing tst_mkfs() API (Cyril)
  We now parse only fs_type, the rest we pass to mkfs.* the same way we
  end up passing them on a command line.

Petr Vorel (6):
  tst_test.sh: Properly init $TST_FS_TYPE
  tst_test.sh: Redesing tst_mkfs() API
  tst_test.sh: Add $TST_FORMAT_DEVICE and related vars
  df01.sh: Use TST_FORMAT_DEVICE=1
  shell: Add test for TST_FORMAT_DEVICE=1
  doc: Add missing shell variables

 doc/shell-test-api.txt                      | 37 +++++++++++++++++++--
 lib/newlib_tests/shell/tst_format_device.sh | 24 +++++++++++++
 testcases/commands/df/df01.sh               |  5 +--
 testcases/commands/mkfs/mkfs01.sh           |  2 +-
 testcases/lib/tst_test.sh                   | 28 ++++++++++------
 5 files changed, 79 insertions(+), 17 deletions(-)
 create mode 100755 lib/newlib_tests/shell/tst_format_device.sh

-- 
2.35.1


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

[LTP] [PATCH v2 1/6] tst_test.sh: Properly init $TST_FS_TYPE

[Petr Vorel]

to sync with C API init $TST_FS_TYPE with $LTP_DEV_FS_TYPE, when
defined. That's equivalent of the .dev_fs_type in the tst_test
structure:

    if (tst_test->dev_fs_type)
	    tdev.fs_type = tst_test->dev_fs_type;
    else
	    tdev.fs_type = tst_dev_fs_type();

    (And the tst_dev_fs_type() returns either $LTP_DEV_FS_TYPE or "ext2"
    if it's not defined.)

Now, when $TST_FS_TYPE properly defined, tst_mkfs() can rely on it
(thus define the check).

Also use the variable in df01.sh, mkfs01.sh (the only shell tests which
works with filesystems).

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/commands/df/df01.sh     | 2 --
 testcases/commands/mkfs/mkfs01.sh | 2 +-
 testcases/lib/tst_test.sh         | 8 ++++----
 3 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/testcases/commands/df/df01.sh b/testcases/commands/df/df01.sh
index b821452e60..e70f33c9b0 100755
--- a/testcases/commands/df/df01.sh
+++ b/testcases/commands/df/df01.sh
@@ -28,8 +28,6 @@ OPTIONS
 EOF
 }
 
-TST_FS_TYPE=ext2
-
 parse_args()
 {
 	TST_FS_TYPE="$2"
diff --git a/testcases/commands/mkfs/mkfs01.sh b/testcases/commands/mkfs/mkfs01.sh
index 17c7fb9e4a..a3ff027a6d 100755
--- a/testcases/commands/mkfs/mkfs01.sh
+++ b/testcases/commands/mkfs/mkfs01.sh
@@ -45,7 +45,7 @@ setup()
 mkfs_verify_type()
 {
 	if [ -z "$1" ]; then
-		blkid $2 -t TYPE="ext2" >/dev/null
+		blkid $2 -t TYPE="$TST_FS_TYPE" >/dev/null
 	else
 		if [ "$1" = "msdos" ]; then
 			blkid $2 -t TYPE="vfat" >/dev/null
diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
index 30614974c3..fa4c90f787 100644
--- a/testcases/lib/tst_test.sh
+++ b/testcases/lib/tst_test.sh
@@ -17,6 +17,10 @@ export TST_ITERATIONS=1
 export TST_TMPDIR_RHOST=0
 export TST_LIB_LOADED=1
 
+if [ -z "$TST_FS_TYPE" ]; then
+	export TST_FS_TYPE="${LTP_DEV_FS_TYPE:-ext2}"
+fi
+
 . tst_ansi_color.sh
 . tst_security.sh
 
@@ -343,10 +347,6 @@ tst_mkfs()
 	[ $# -ge 1 ] && shift
 	local fs_opts="$@"
 
-	if [ -z "$fs_type" ]; then
-		tst_brk TBROK "No fs_type specified"
-	fi
-
 	if [ -z "$device" ]; then
 		tst_brk TBROK "No device specified"
 	fi
-- 
2.35.1


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

[LTP] [PATCH v2 2/6] tst_test.sh: Redesing tst_mkfs() API

[Petr Vorel]

the goal is to add support for extra opts after device name
(e.g. size) - to sync with C API.

But instead adding new variable for extra opts we specify only
$fs_type and the rest of args we just pass to the mkfs.* command.
Still keep $TST_FS_TYPE and $TST_DEVICE as a default when no parameters
are passed.

NOTE: not adding it to legacy shell API (thus no need to change
test_robind.sh).

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/lib/tst_test.sh | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
index fa4c90f787..64e65f9f53 100644
--- a/testcases/lib/tst_test.sh
+++ b/testcases/lib/tst_test.sh
@@ -342,19 +342,21 @@ tst_umount()
 tst_mkfs()
 {
 	local fs_type=${1:-$TST_FS_TYPE}
-	local device=${2:-$TST_DEVICE}
 	[ $# -ge 1 ] && shift
-	[ $# -ge 1 ] && shift
-	local fs_opts="$@"
 
-	if [ -z "$device" ]; then
-		tst_brk TBROK "No device specified"
+	local opts="$@"
+
+	if [ -z "$opts" ]; then
+		if [ "$TST_NEEDS_DEVICE" != 1 ]; then
+			tst_brk "Using default parameters in tst_mkfs requires TST_NEEDS_DEVICE=1"
+		fi
+		opts="$TST_DEVICE"
 	fi
 
 	tst_require_cmds mkfs.$fs_type
 
-	tst_res TINFO "Formatting $device with $fs_type extra opts='$fs_opts'"
-	ROD_SILENT mkfs.$fs_type $fs_opts $device
+	tst_res TINFO "Formatting $fs_type with opts='$opts'"
+	ROD_SILENT mkfs.$fs_type $opts
 }
 
 # Detect whether running under hypervisor: Microsoft Hyper-V
-- 
2.35.1


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

[LTP] [PATCH v2 3/6] tst_test.sh: Add $TST_FORMAT_DEVICE and related vars

[Petr Vorel]

i.e.: TST_DEV_EXTRA_OPTS, TST_DEV_FS_OPTS, TST_FS_TYPE
(to sync with C API).

NOTE: stop using pattern 'TST_NEEDS_FOO', sometimes 'NEEDS' just does
not fit to variable name.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 doc/shell-test-api.txt    | 31 +++++++++++++++++++++++++++++--
 testcases/lib/tst_test.sh |  6 ++++++
 2 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/doc/shell-test-api.txt b/doc/shell-test-api.txt
index a5974b4fad..5caa889c9e 100644
--- a/doc/shell-test-api.txt
+++ b/doc/shell-test-api.txt
@@ -189,11 +189,20 @@ space as default value is used. Of course, it's possible to use separate functio
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 Similarily to the C library various checks and preparations can be requested
-simply by setting right '$TST_NEEDS_FOO'.
+simply by setting right '$TST_FOO'.
 
 [options="header"]
 |=============================================================================
 | Variable name            | Action done
+| 'TST_DEV_EXTRA_OPTS'     | Pass extra 'mkfs' options _after_ device name,
+|                          | to 'tst_mkfs', use with 'TST_FORMAT_DEVICE=1'.
+| 'TST_DEV_FS_OPTS'        | Pass 'mkfs' options _before_ the device name,
+|                          | to 'tst_mkfs', use with 'TST_FORMAT_DEVICE=1'.
+| 'TST_FORMAT_DEVICE'      | Format a block device with a filesystem, see
+|                          | https://github.com/linux-test-project/ltp/wiki/Shell-Test-API#formatting-device-with-a-filesystem[Formatting device with a filesystem]
+|                          | See also 'TST_DEV_EXTRA_OPTS', 'TST_DEV_FS_OPTS', 'TST_FS_TYPE'.
+|                          | Implies 'TST_NEEDS_DEVICE=1' (no need to set it).
+| 'TST_FS_TYPE'            | Override the default filesystem to be used.
 | 'TST_NEEDS_ROOT'         | Exit the test with 'TCONF' unless executed under root.
 |                          | Alternatively the 'tst_require_root' command can be used.
 | 'TST_NEEDS_TMPDIR'       | Create test temporary directory and cd into it.
@@ -495,7 +504,25 @@ tst_random 0 1000
 Formatting device with a filesystem
 +++++++++++++++++++++++++++++++++++
 
-The 'tst_mkfs' helper will format device with the filesystem.
+'TST_FORMAT_DEVICE=1' can be used to format device before running the test.
+Uses '$TST_FS_TYPE' (used filesystem, by default ext2), '$TST_DEVICE' (used
+block device, usually prepared by 'TST_NEEDS_DEVICE=1'), '$TST_DEV_FS_OPTS'
+('mkfs' options _before_ the device name) and '$TST_DEV_EXTRA_OPTS'
+(extra 'mkfs' options _after_ the device name).
+Library internally uses 'tst_mkfs' function, which can be used for more complex setup.
+
+[source,sh]
+-------------------------------------------------------------------------------
+TST_FORMAT_DEVICE=1
+TST_DEV_FS_OPTS="-b 1024 -O quota"
+TST_DEV_EXTRA_OPTS="5m"
+TST_TESTFUNC=test
+
+test1()
+{
+	tst_res TPASS "device formatted"
+}
+-------------------------------------------------------------------------------
 
 [source,sh]
 -------------------------------------------------------------------------------
diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
index 64e65f9f53..0968391984 100644
--- a/testcases/lib/tst_test.sh
+++ b/testcases/lib/tst_test.sh
@@ -616,6 +616,7 @@ tst_run()
 			NET_SKIP_VARIABLE_INIT|NEEDS_CHECKPOINTS);;
 			CHECKPOINT_WAIT|CHECKPOINT_WAKE);;
 			CHECKPOINT_WAKE2|CHECKPOINT_WAKE_AND_WAIT);;
+			DEV_EXTRA_OPTS|DEV_FS_OPTS|FORMAT_DEVICE);;
 			*) tst_res TWARN "Reserved variable TST_$_tst_i used!";;
 			esac
 		done
@@ -660,6 +661,7 @@ tst_run()
 
 	_tst_setup_timer
 
+	[ "$TST_FORMAT_DEVICE" = 1 ] && TST_NEEDS_DEVICE=1
 	[ "$TST_NEEDS_DEVICE" = 1 ] && TST_NEEDS_TMPDIR=1
 
 	if [ "$TST_NEEDS_TMPDIR" = 1 ]; then
@@ -691,6 +693,10 @@ tst_run()
 
 	[ -n "$TST_NEEDS_MODULE" ] && tst_require_module "$TST_NEEDS_MODULE"
 
+	if [ "$TST_FORMAT_DEVICE" = 1 ]; then
+		tst_mkfs $TST_FS_TYPE $TST_DEV_FS_OPTS $TST_DEVICE $TST_DEV_EXTRA_OPTS
+	fi
+
 	[ -n "$TST_NEEDS_CHECKPOINTS" ] && _tst_init_checkpoints
 
 	if [ -n "$TST_SETUP" ]; then
-- 
2.35.1


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

[LTP] [PATCH v2 4/6] df01.sh: Use TST_FORMAT_DEVICE=1

[Petr Vorel]

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Same as in v1

 testcases/commands/df/df01.sh | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/testcases/commands/df/df01.sh b/testcases/commands/df/df01.sh
index e70f33c9b0..86df940861 100755
--- a/testcases/commands/df/df01.sh
+++ b/testcases/commands/df/df01.sh
@@ -13,7 +13,7 @@ TST_OPTS="f:"
 TST_USAGE=usage
 TST_PARSE_ARGS=parse_args
 TST_NEEDS_ROOT=1
-TST_NEEDS_DEVICE=1
+TST_FORMAT_DEVICE=1
 . tst_test.sh
 
 usage()
@@ -35,7 +35,6 @@ parse_args()
 
 setup()
 {
-	tst_mkfs
 	tst_mount
 	DF_FS_TYPE=$(mount | grep "$TST_DEVICE" | awk 'NR==1{print $5}')
 }
-- 
2.35.1


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

[LTP] [PATCH v2 5/6] shell: Add test for TST_FORMAT_DEVICE=1

[Petr Vorel]

Unfortunately GitHub Actions don't have loop devices, thus cannot be run
in CI:

tst_format_device 1 TINFO: timeout per run is 0h 5m 0s
/__w/ltp/ltp/lib/tst_device.c:139: TINFO: No free devices found

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Same as in v1

 lib/newlib_tests/shell/tst_format_device.sh | 24 +++++++++++++++++++++
 1 file changed, 24 insertions(+)
 create mode 100755 lib/newlib_tests/shell/tst_format_device.sh

diff --git a/lib/newlib_tests/shell/tst_format_device.sh b/lib/newlib_tests/shell/tst_format_device.sh
new file mode 100755
index 0000000000..b02f8b122b
--- /dev/null
+++ b/lib/newlib_tests/shell/tst_format_device.sh
@@ -0,0 +1,24 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) 2022 Petr Vorel <pvorel@suse.cz>
+
+TST_FORMAT_DEVICE=1
+TST_TESTFUNC=test
+TST_CNT=2
+TST_DEV_FS_OPTS="-b 1024 -O quota"
+TST_DEV_EXTRA_OPTS="5m"
+TST_NEEDS_CMDS="df lsblk"
+. tst_test.sh
+
+test1()
+{
+	tst_res TPASS "device formatted"
+}
+
+test2()
+{
+	tst_check_cmds df || return
+	EXPECT_PASS "df $TST_DEVICE | grep -q /dev"
+}
+
+tst_run
-- 
2.35.1


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

[LTP] [PATCH v2 6/6] doc: Add missing shell variables

[Petr Vorel]

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 doc/shell-test-api.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/doc/shell-test-api.txt b/doc/shell-test-api.txt
index 5caa889c9e..7172917e05 100644
--- a/doc/shell-test-api.txt
+++ b/doc/shell-test-api.txt
@@ -202,7 +202,13 @@ simply by setting right '$TST_FOO'.
 |                          | https://github.com/linux-test-project/ltp/wiki/Shell-Test-API#formatting-device-with-a-filesystem[Formatting device with a filesystem]
 |                          | See also 'TST_DEV_EXTRA_OPTS', 'TST_DEV_FS_OPTS', 'TST_FS_TYPE'.
 |                          | Implies 'TST_NEEDS_DEVICE=1' (no need to set it).
+| 'TST_DEVICE'             | Block device name for 'tst_mount' and 'tst_mkfs', see
+|                          | https://github.com/linux-test-project/ltp/wiki/Shell-Test-API#formatting-device-with-a-filesystem[Formatting device with a filesystem]
 | 'TST_FS_TYPE'            | Override the default filesystem to be used.
+| 'TST_MNTPOINT'           | Holds path to mountpoint when use 'tst_mount', see
+|                          | https://github.com/linux-test-project/ltp/wiki/Shell-Test-API#formatting-device-with-a-filesystem[Formatting device with a filesystem]
+| 'TST_MNT_PARAMS'         | Extra mount params for 'tst_mount', see
+|                          | https://github.com/linux-test-project/ltp/wiki/Shell-Test-API#formatting-device-with-a-filesystem[Formatting device with a filesystem]
 | 'TST_NEEDS_ROOT'         | Exit the test with 'TCONF' unless executed under root.
 |                          | Alternatively the 'tst_require_root' command can be used.
 | 'TST_NEEDS_TMPDIR'       | Create test temporary directory and cd into it.
-- 
2.35.1


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

Re: [LTP] [PATCH v2 5/6] shell: Add test for TST_FORMAT_DEVICE=1

[xuyang2018.jy]

Hi Petr
> Unfortunately GitHub Actions don't have loop devices, thus cannot be run
> in CI:
> 
> tst_format_device 1 TINFO: timeout per run is 0h 5m 0s
> /__w/ltp/ltp/lib/tst_device.c:139: TINFO: No free devices found
> 
> Signed-off-by: Petr Vorel<pvorel@suse.cz>
> ---
> Same as in v1
> 
>   lib/newlib_tests/shell/tst_format_device.sh | 24 +++++++++++++++++++++
>   1 file changed, 24 insertions(+)
>   create mode 100755 lib/newlib_tests/shell/tst_format_device.sh
> 
> diff --git a/lib/newlib_tests/shell/tst_format_device.sh b/lib/newlib_tests/shell/tst_format_device.sh
> new file mode 100755
> index 0000000000..b02f8b122b
> --- /dev/null
> +++ b/lib/newlib_tests/shell/tst_format_device.sh
> @@ -0,0 +1,24 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# Copyright (c) 2022 Petr Vorel<pvorel@suse.cz>
> +
> +TST_FORMAT_DEVICE=1
> +TST_TESTFUNC=test
> +TST_CNT=2
> +TST_DEV_FS_OPTS="-b 1024 -O quota"
> +TST_DEV_EXTRA_OPTS="5m"
> +TST_NEEDS_CMDS="df lsblk"
> +. tst_test.sh
> +
> +test1()
> +{
> +	tst_res TPASS "device formatted"
> +}
> +
> +test2()
> +{
> +	tst_check_cmds df || return
We have checked df command in TST_NEEDS_CMDS, why still check it again
in here?

Best Regards
Yang Xu
> +	EXPECT_PASS "df $TST_DEVICE | grep -q /dev"
> +}
> +
> +tst_run

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

Re: [LTP] [PATCH v2 6/6] doc: Add missing shell variables

[xuyang2018.jy]

Hi Petr

This patchset looks good to me, so
Acked-by: Yang Xu <xuyang2018.jy@fujitsu.com>

ps: I think we can add two basic tests for this,
1)test TST_FS_TYPE whether format the specified filesystem ie using ext4
2)test tst_mkfs whether format failed if not specifying TST_NEEDS_DEVICE
and call tst_mkfs directly

Best Regards
Yang Xu
> Signed-off-by: Petr Vorel<pvorel@suse.cz>
> ---
>   doc/shell-test-api.txt | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/doc/shell-test-api.txt b/doc/shell-test-api.txt
> index 5caa889c9e..7172917e05 100644
> --- a/doc/shell-test-api.txt
> +++ b/doc/shell-test-api.txt
> @@ -202,7 +202,13 @@ simply by setting right '$TST_FOO'.
>   |                          | https://github.com/linux-test-project/ltp/wiki/Shell-Test-API#formatting-device-with-a-filesystem[Formatting device with a filesystem]
>   |                          | See also 'TST_DEV_EXTRA_OPTS', 'TST_DEV_FS_OPTS', 'TST_FS_TYPE'.
>   |                          | Implies 'TST_NEEDS_DEVICE=1' (no need to set it).
> +| 'TST_DEVICE'             | Block device name for 'tst_mount' and 'tst_mkfs', see
> +|                          | https://github.com/linux-test-project/ltp/wiki/Shell-Test-API#formatting-device-with-a-filesystem[Formatting device with a filesystem]
>   | 'TST_FS_TYPE'            | Override the default filesystem to be used.
> +| 'TST_MNTPOINT'           | Holds path to mountpoint when use 'tst_mount', see
> +|                          | https://github.com/linux-test-project/ltp/wiki/Shell-Test-API#formatting-device-with-a-filesystem[Formatting device with a filesystem]
> +| 'TST_MNT_PARAMS'         | Extra mount params for 'tst_mount', see
> +|                          | https://github.com/linux-test-project/ltp/wiki/Shell-Test-API#formatting-device-with-a-filesystem[Formatting device with a filesystem]
>   | 'TST_NEEDS_ROOT'         | Exit the test with 'TCONF' unless executed under root.
>   |                          | Alternatively the 'tst_require_root' command can be used.
>   | 'TST_NEEDS_TMPDIR'       | Create test temporary directory and cd into it.

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

Re: [LTP] [PATCH v2 5/6] shell: Add test for TST_FORMAT_DEVICE=1

[Petr Vorel]

Hi Xu,

> >   lib/newlib_tests/shell/tst_format_device.sh | 24 +++++++++++++++++++++
...
> > +TST_FORMAT_DEVICE=1
> > +TST_TESTFUNC=test
> > +TST_CNT=2
> > +TST_DEV_FS_OPTS="-b 1024 -O quota"
> > +TST_DEV_EXTRA_OPTS="5m"
> > +TST_NEEDS_CMDS="df lsblk"
This is left over from previous versions => should be deleted.
> > +. tst_test.sh
> > +
> > +test1()
> > +{
> > +	tst_res TPASS "device formatted"
> > +}
> > +
> > +test2()
> > +{
> > +	tst_check_cmds df || return
> We have checked df command in TST_NEEDS_CMDS, why still check it again
> in here?
Correct, TST_NEEDS_CMDS should be removed so that at least creating the device
(test1) is supported even SUT is missing df.

Kind regards,
Petr

> Best Regards
> Yang Xu

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

Re: [LTP] [PATCH v2 3/6] tst_test.sh: Add $TST_FORMAT_DEVICE and related vars

[xuyang2018.jy]

Hi Petr
> +
> +[source,sh]
> +-------------------------------------------------------------------------------
> +TST_FORMAT_DEVICE=1
> +TST_DEV_FS_OPTS="-b 1024 -O quota"
> +TST_DEV_EXTRA_OPTS="5m"
> +TST_TESTFUNC=test
> +
> +test1()
A small nit, I prefer to use test name directly if we don't have several
sub test cases.

Best Regards
Yang Xu
> +{
> +	tst_res TPASS "device formatted"
> +}

> +-------------------------------------------------------------------------------

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

Re: [LTP] [PATCH v2 3/6] tst_test.sh: Add $TST_FORMAT_DEVICE and related vars

[Petr Vorel]

Hi Xu,
> Hi Petr
> > +
> > +[source,sh]
> > +-------------------------------------------------------------------------------
> > +TST_FORMAT_DEVICE=1
> > +TST_DEV_FS_OPTS="-b 1024 -O quota"
> > +TST_DEV_EXTRA_OPTS="5m"
> > +TST_TESTFUNC=test
> > +
> > +test1()
> A small nit, I prefer to use test name directly if we don't have several
> sub test cases.
Sure. Originally I had more tests there, using lsblk.
But I'll probably extend these tests to your suggestion in different mail.

Kind regards,
Petr

> Best Regards
> Yang Xu
> > +{
> > +	tst_res TPASS "device formatted"
> > +}

> > +-------------------------------------------------------------------------------

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

Re: [LTP] [PATCH v2 6/6] doc: Add missing shell variables

[Petr Vorel]

Hi Xu,

> Hi Petr

> This patchset looks good to me, so
> Acked-by: Yang Xu <xuyang2018.jy@fujitsu.com>

> ps: I think we can add two basic tests for this,
> 1)test TST_FS_TYPE whether format the specified filesystem ie using ext4
BTW the default is ext2. Also not sure how to do it when formatting loop device.
Verifying filesystem I'll add when I add TST_NEEDS_MOUNT flag.

losetup prints:
/dev/loop0: [65025]:176189 (/tmp/LTP_tst_format_device.9bya3blyQD/test_dev.img)

> 2)test tst_mkfs whether format failed if not specifying TST_NEEDS_DEVICE
> and call tst_mkfs directly
+1. I'll also document here what is the purpose (we have many tests which are
expected to TFAIL/TBROK, but it's not documented in the tests thus it's hard to
run them locally.

Kind regards,
Petr

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

Re: [LTP] [PATCH v2 1/6] tst_test.sh: Properly init $TST_FS_TYPE

[Cyril Hrubis]

Hi!
Looks good.

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v2 2/6] tst_test.sh: Redesing tst_mkfs() API

[Cyril Hrubis]

Hi!
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v2 3/6] tst_test.sh: Add $TST_FORMAT_DEVICE and related vars

[Cyril Hrubis]

Hi!
> ---
>  doc/shell-test-api.txt    | 31 +++++++++++++++++++++++++++++--
>  testcases/lib/tst_test.sh |  6 ++++++
>  2 files changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/doc/shell-test-api.txt b/doc/shell-test-api.txt
> index a5974b4fad..5caa889c9e 100644
> --- a/doc/shell-test-api.txt
> +++ b/doc/shell-test-api.txt
> @@ -189,11 +189,20 @@ space as default value is used. Of course, it's possible to use separate functio
>  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>  
>  Similarily to the C library various checks and preparations can be requested
> -simply by setting right '$TST_NEEDS_FOO'.
> +simply by setting right '$TST_FOO'.
>  
>  [options="header"]
>  |=============================================================================
>  | Variable name            | Action done
> +| 'TST_DEV_EXTRA_OPTS'     | Pass extra 'mkfs' options _after_ device name,
> +|                          | to 'tst_mkfs', use with 'TST_FORMAT_DEVICE=1'.
> +| 'TST_DEV_FS_OPTS'        | Pass 'mkfs' options _before_ the device name,
> +|                          | to 'tst_mkfs', use with 'TST_FORMAT_DEVICE=1'.
> +| 'TST_FORMAT_DEVICE'      | Format a block device with a filesystem, see
> +|                          | https://github.com/linux-test-project/ltp/wiki/Shell-Test-API#formatting-device-with-a-filesystem[Formatting device with a filesystem]

Hm, I guess that we already have a few absolute links in the docs to a
github.com wiki, but I do not like it that much. I guess that realtive
links does not work, right?

> +|                          | See also 'TST_DEV_EXTRA_OPTS', 'TST_DEV_FS_OPTS', 'TST_FS_TYPE'.
> +|                          | Implies 'TST_NEEDS_DEVICE=1' (no need to set it).
> +| 'TST_FS_TYPE'            | Override the default filesystem to be used.
>  | 'TST_NEEDS_ROOT'         | Exit the test with 'TCONF' unless executed under root.
>  |                          | Alternatively the 'tst_require_root' command can be used.
>  | 'TST_NEEDS_TMPDIR'       | Create test temporary directory and cd into it.
> @@ -495,7 +504,25 @@ tst_random 0 1000
>  Formatting device with a filesystem
>  +++++++++++++++++++++++++++++++++++
>  
> -The 'tst_mkfs' helper will format device with the filesystem.
> +'TST_FORMAT_DEVICE=1' can be used to format device before running the test.
> +Uses '$TST_FS_TYPE' (used filesystem, by default ext2), '$TST_DEVICE' (used
                              ^
			      I would removed this part and keep only
			      the (by default ext2)
> +block device, usually prepared by 'TST_NEEDS_DEVICE=1'), '$TST_DEV_FS_OPTS'

and rewrote this part no to be in the parentheses as:

'$TST_DEVICE' a block device to be formatted, usually prepared by the
library (TST_NEEDS_DEVICE=1 must be set).

> +('mkfs' options _before_ the device name) and '$TST_DEV_EXTRA_OPTS'
> +(extra 'mkfs' options _after_ the device name).

And remove the parentheses here as:

'$TST_DEV_FS_OPTS' a 'mkfs' options _before_ the device path and
'$TST_DEV_EXTRA_OPTS' extra 'mkfs'' options _after_ the device path.


Other than that and the test1() pointed out by Yang Xu:

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v2 4/6] df01.sh: Use TST_FORMAT_DEVICE=1

[Cyril Hrubis]

Hi!
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v2 5/6] shell: Add test for TST_FORMAT_DEVICE=1

[Cyril Hrubis]

Hi!
> > > +TST_FORMAT_DEVICE=1
> > > +TST_TESTFUNC=test
> > > +TST_CNT=2
> > > +TST_DEV_FS_OPTS="-b 1024 -O quota"
> > > +TST_DEV_EXTRA_OPTS="5m"
> > > +TST_NEEDS_CMDS="df lsblk"
> This is left over from previous versions => should be deleted.
> > > +. tst_test.sh
> > > +
> > > +test1()
> > > +{
> > > +	tst_res TPASS "device formatted"
> > > +}
> > > +
> > > +test2()
> > > +{
> > > +	tst_check_cmds df || return
> > We have checked df command in TST_NEEDS_CMDS, why still check it again
> > in here?
> Correct, TST_NEEDS_CMDS should be removed so that at least creating the device
> (test1) is supported even SUT is missing df.

And what do we need the lsblk command for?

Other than the TST_NEEDS_CMD= variable:

Reviwed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v2 6/6] doc: Add missing shell variables

[Cyril Hrubis]

Hi!
> diff --git a/doc/shell-test-api.txt b/doc/shell-test-api.txt
> index 5caa889c9e..7172917e05 100644
> --- a/doc/shell-test-api.txt
> +++ b/doc/shell-test-api.txt
> @@ -202,7 +202,13 @@ simply by setting right '$TST_FOO'.
>  |                          | https://github.com/linux-test-project/ltp/wiki/Shell-Test-API#formatting-device-with-a-filesystem[Formatting device with a filesystem]
>  |                          | See also 'TST_DEV_EXTRA_OPTS', 'TST_DEV_FS_OPTS', 'TST_FS_TYPE'.
>  |                          | Implies 'TST_NEEDS_DEVICE=1' (no need to set it).
> +| 'TST_DEVICE'             | Block device name for 'tst_mount' and 'tst_mkfs', see
> +|                          | https://github.com/linux-test-project/ltp/wiki/Shell-Test-API#formatting-device-with-a-filesystem[Formatting device with a filesystem]
>  | 'TST_FS_TYPE'            | Override the default filesystem to be used.
> +| 'TST_MNTPOINT'           | Holds path to mountpoint when use 'tst_mount', see
                                                         ^
							 used in 'tst_mount'
> +|                          | https://github.com/linux-test-project/ltp/wiki/Shell-Test-API#formatting-device-with-a-filesystem[Formatting device with a filesystem]
> +| 'TST_MNT_PARAMS'         | Extra mount params for 'tst_mount', see
> +|                          | https://github.com/linux-test-project/ltp/wiki/Shell-Test-API#formatting-device-with-a-filesystem[Formatting device with a filesystem]
>  | 'TST_NEEDS_ROOT'         | Exit the test with 'TCONF' unless executed under root.
>  |                          | Alternatively the 'tst_require_root' command can be used.
>  | 'TST_NEEDS_TMPDIR'       | Create test temporary directory and cd into it.

Other than the minor correction:

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v2 3/6] tst_test.sh: Add $TST_FORMAT_DEVICE and related vars

[Petr Vorel]

Hi Cyril,

> > +| 'TST_FORMAT_DEVICE'      | Format a block device with a filesystem, see
> > +|                          | https://github.com/linux-test-project/ltp/wiki/Shell-Test-API#formatting-device-with-a-filesystem[Formatting device with a filesystem]

> Hm, I guess that we already have a few absolute links in the docs to a
> github.com wiki, but I do not like it that much. I guess that realtive
> links does not work, right?

AFAIK it does not in AsciiDoc format (not sure about other, but we don't want to
switch anyway). I guess I can keep it as we don't have any better solution,
right?

> > +|                          | See also 'TST_DEV_EXTRA_OPTS', 'TST_DEV_FS_OPTS', 'TST_FS_TYPE'.
> > +|                          | Implies 'TST_NEEDS_DEVICE=1' (no need to set it).
> > +| 'TST_FS_TYPE'            | Override the default filesystem to be used.
> >  | 'TST_NEEDS_ROOT'         | Exit the test with 'TCONF' unless executed under root.
> >  |                          | Alternatively the 'tst_require_root' command can be used.
> >  | 'TST_NEEDS_TMPDIR'       | Create test temporary directory and cd into it.
> > @@ -495,7 +504,25 @@ tst_random 0 1000
> >  Formatting device with a filesystem
> >  +++++++++++++++++++++++++++++++++++

> > -The 'tst_mkfs' helper will format device with the filesystem.
> > +'TST_FORMAT_DEVICE=1' can be used to format device before running the test.
> > +Uses '$TST_FS_TYPE' (used filesystem, by default ext2), '$TST_DEVICE' (used
>                               ^
> 			      I would removed this part and keep only
> 			      the (by default ext2)
> > +block device, usually prepared by 'TST_NEEDS_DEVICE=1'), '$TST_DEV_FS_OPTS'

> and rewrote this part no to be in the parentheses as:

> '$TST_DEVICE' a block device to be formatted, usually prepared by the
> library (TST_NEEDS_DEVICE=1 must be set).

> > +('mkfs' options _before_ the device name) and '$TST_DEV_EXTRA_OPTS'
> > +(extra 'mkfs' options _after_ the device name).

> And remove the parentheses here as:

> '$TST_DEV_FS_OPTS' a 'mkfs' options _before_ the device path and
> '$TST_DEV_EXTRA_OPTS' extra 'mkfs'' options _after_ the device path.

Agree with all of this.


> Other than that and the test1() pointed out by Yang Xu:
I suppose this is what you mean: s/test1/test/, right?

+++ doc/shell-test-api.txt
@@ -517,7 +517,7 @@ TST_DEV_FS_OPTS="-b 1024 -O quota"
 TST_DEV_EXTRA_OPTS="5m"
 TST_TESTFUNC=test

-test1()
+test()
 {
        tst_res TPASS "device formatted"
 }


> Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
Thanks!

Kind regards,
Petr

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

Re: [LTP] [PATCH v2 6/6] doc: Add missing shell variables

[Petr Vorel]

Hi Xu, Cyril,

whole patchset merged with fixes you pointed out and your tags.

Hope we find some better solution for relative path links.

Thanks for your review.

Kind regards,
Petr

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