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

[Petr Vorel <pvorel@suse.cz>]

> Hi Cyril,

> > > > +++ 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.
> > Good catch, I forget to check examples in docs.

> > -T largefile would have to be quoted:
> > tst_mkfs ext4 /dev/device "-T largefile"


> > > 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 $@.

> > But then you'd have to always pass a device ($TST_DEVICE).
> > And the problem is that I wanted to specify the options before loading
> > tst_test.sh (together with TST_FORMAT_DEVICE=1). At this point TST_DEVICE
> > is empty string. Therefore I'd prefer to quote fs-options to be passed to
> > tst_mkfs as single option.
> OK, now I understand you. It's all about tst_mkfs having $1, shift and rest $@.
> And we avoid quoting as you suggested :).

> But TST_* variables in stay the same. => I'll implement it in v2.

Cyril, we forget on think: when passing everything as $@, we just don't know
what is the device.

Thus I'll change
tst_res TINFO "Formatting $device with $fs_type opts='$fs_opts' extra opts='$extra_opts'"
with
tst_res TINFO "Formatting $fs_type with opts='$opts'"

It's just a minor logging problem.
We also loose check for missing device, that's not ideal as it's mandatory, thus
I'll add some check when using the default $TST_DEVICE.

Kind regards,
Petr

> Thanks!

> Kind regards,
> Petr

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