Re: [LTP] [PATCH 2/5] tst_test.sh: tst_mkfs(): Add support for extra opts

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> Allow to pass options after device name
> (to sync with C API).
> 
> 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 | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
> index eb3bf0df12..4707613158 100644
> --- a/testcases/lib/tst_test.sh
> +++ b/testcases/lib/tst_test.sh
> @@ -343,9 +343,8 @@ tst_mkfs()
>  	[ -z "$fs_type" ] && fs_type="$TST_DEFAULT_FS_TYPE"
>  
>  	local device=${2:-$TST_DEVICE}
> -	[ $# -ge 1 ] && shift
> -	[ $# -ge 1 ] && shift
> -	local fs_opts="$@"
> +	local fs_opts="$3"
> +	local extra_opts="$4"

Doesn't this actually break the examples described in the
doc/shell-test-api.txt?

There we do have:

tst_mkfs ext4 /dev/device -T largefile

Which obviously parses as "-T" in the fs_opts and "largefile" in the
extra_opts.

I do not think that the support for extra opts can be added this way
without redesigning the tst_mkfs API. Maybe it would make more sense to
just pass the parameters in the same way we end up passing them on a
command line, i.e. we would only interpret the first parameter passed to
tst_mkfs as a filesystem type and pass the rest as $@.

>  	if [ -z "$device" ]; then
>  		tst_brk TBROK "No device specified"
> @@ -353,8 +352,8 @@ tst_mkfs()
>  
>  	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 $device with $fs_type opts='$fs_opts' extra opts='$extra_opts'"
> +	ROD_SILENT mkfs.$fs_type $fs_opts $device $extra_opts
>  }
>  
>  # Detect whether running under hypervisor: Microsoft Hyper-V
> -- 
> 2.34.1
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

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