* [LTP] [PATCH 1/1] net/sendfile01.sh: Check with timeout
@ 2020-04-21 18:00 Petr Vorel
2020-04-23 19:04 ` Alexey Kodanev
0 siblings, 1 reply; 8+ messages in thread
From: Petr Vorel @ 2020-04-21 18:00 UTC (permalink / raw)
To: ltp
Suggested-by: Alexey Kodanev <alexey.kodanev@oracle.com>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Hi Alexey,
I guess nothing controversial here as failure of starting server is
guarded by -s.
I was thinking about using TST_RETRY_FUNC, but passing command to it
leads to: tst_rhost_run: unknown option: l
What bothers me more, that TST_NEEDS_CMDS does not check command on
rhost. Do we want to have something like TST_NEEDS_CMDS_RHOST or we just
don't care?
Kind regards,
Petr
testcases/network/tcp_cmds/sendfile/sendfile01.sh | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/testcases/network/tcp_cmds/sendfile/sendfile01.sh b/testcases/network/tcp_cmds/sendfile/sendfile01.sh
index af9cadf57..94fff2c9a 100755
--- a/testcases/network/tcp_cmds/sendfile/sendfile01.sh
+++ b/testcases/network/tcp_cmds/sendfile/sendfile01.sh
@@ -11,7 +11,7 @@ TST_SETUP=do_setup
TST_CLEANUP=do_cleanup
TST_TESTFUNC=do_test
TST_NEEDS_TMPDIR=1
-TST_NEEDS_CMDS="diff stat"
+TST_NEEDS_CMDS="diff ss stat"
. tst_net.sh
do_setup()
@@ -28,7 +28,10 @@ do_setup()
tst_rhost_run -s -b -c "$server $(tst_ipaddr rhost) $port"
server_started=1
tst_res TINFO "wait for the server to start"
- sleep 1
+ while true; do
+ tst_rhost_run -c "ss -ltp" | grep -q "$port.*testsf" && break
+ tst_sleep 10ms
+ done
}
do_test()
--
2.26.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [LTP] [PATCH 1/1] net/sendfile01.sh: Check with timeout
2020-04-21 18:00 [LTP] [PATCH 1/1] net/sendfile01.sh: Check with timeout Petr Vorel
@ 2020-04-23 19:04 ` Alexey Kodanev
2020-04-23 22:12 ` Petr Vorel
2020-04-24 12:53 ` Petr Vorel
0 siblings, 2 replies; 8+ messages in thread
From: Alexey Kodanev @ 2020-04-23 19:04 UTC (permalink / raw)
To: ltp
On 21.04.2020 21:00, Petr Vorel wrote:
> Suggested-by: Alexey Kodanev <alexey.kodanev@oracle.com>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> Hi Alexey,
>
> I guess nothing controversial here as failure of starting server is
> guarded by -s.
> I was thinking about using TST_RETRY_FUNC, but passing command to it
> leads to: tst_rhost_run: unknown option: l
>
Hi Petr,
eval might help in this case, take a look at tst_retry() in test.sh
old api, not sure why exactly it was removed in the new one...
index 1d8a71d9f..e34edb26a 100644
--- a/testcases/lib/tst_test.sh
+++ b/testcases/lib/tst_test.sh
@@ -207,7 +207,7 @@ TST_RETRY_FN_EXP_BACKOFF()
fi
while true; do
- $tst_fun
+ eval "$tst_fun"
if [ "$?" = "$tst_exp" ]; then
break
fi
> What bothers me more, that TST_NEEDS_CMDS does not check command on
> rhost. Do we want to have something like TST_NEEDS_CMDS_RHOST or we just
> don't care?
In general, yes, we need to check if a command exists on the remote host.
Yet another variable, what about checking what in TST_NEEDS_CMDS on the
remote host? Though TST_NEEDS_CMDS_RHOST looks quite well.
>
> Kind regards,
> Petr
>
> testcases/network/tcp_cmds/sendfile/sendfile01.sh | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/testcases/network/tcp_cmds/sendfile/sendfile01.sh b/testcases/network/tcp_cmds/sendfile/sendfile01.sh
> index af9cadf57..94fff2c9a 100755
> --- a/testcases/network/tcp_cmds/sendfile/sendfile01.sh
> +++ b/testcases/network/tcp_cmds/sendfile/sendfile01.sh
> @@ -11,7 +11,7 @@ TST_SETUP=do_setup
> TST_CLEANUP=do_cleanup
> TST_TESTFUNC=do_test
> TST_NEEDS_TMPDIR=1
> -TST_NEEDS_CMDS="diff stat"
> +TST_NEEDS_CMDS="diff ss stat"
> . tst_net.sh
>
> do_setup()
> @@ -28,7 +28,10 @@ do_setup()
> tst_rhost_run -s -b -c "$server $(tst_ipaddr rhost) $port"
> server_started=1
> tst_res TINFO "wait for the server to start"
> - sleep 1
> + while true; do
> + tst_rhost_run -c "ss -ltp" | grep -q "$port.*testsf" && break
> + tst_sleep 10ms
> + done
> }
>
> do_test()
>
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [LTP] [PATCH 1/1] net/sendfile01.sh: Check with timeout
2020-04-23 19:04 ` Alexey Kodanev
@ 2020-04-23 22:12 ` Petr Vorel
2020-04-24 12:11 ` Petr Vorel
2020-04-24 12:53 ` Petr Vorel
1 sibling, 1 reply; 8+ messages in thread
From: Petr Vorel @ 2020-04-23 22:12 UTC (permalink / raw)
To: ltp
Hi Alexey,
> > I guess nothing controversial here as failure of starting server is
> > guarded by -s.
> > I was thinking about using TST_RETRY_FUNC, but passing command to it
> > leads to: tst_rhost_run: unknown option: l
> Hi Petr,
> eval might help in this case, take a look at tst_retry() in test.sh
Good point.
> old api, not sure why exactly it was removed in the new one...
It was designed from scratch I guess.
But this patch makes sense to me, I'll test it tomorrow.
> index 1d8a71d9f..e34edb26a 100644
> --- a/testcases/lib/tst_test.sh
> +++ b/testcases/lib/tst_test.sh
> @@ -207,7 +207,7 @@ TST_RETRY_FN_EXP_BACKOFF()
> fi
> while true; do
> - $tst_fun
> + eval "$tst_fun"
> if [ "$?" = "$tst_exp" ]; then
> break
> fi
> > What bothers me more, that TST_NEEDS_CMDS does not check command on
> > rhost. Do we want to have something like TST_NEEDS_CMDS_RHOST or we just
> > don't care?
> In general, yes, we need to check if a command exists on the remote host.
> Yet another variable, what about checking what in TST_NEEDS_CMDS on the
> remote host? Though TST_NEEDS_CMDS_RHOST looks quite well.
In this case ss was needed only on rhost.
I don't know if SUT in two hosts configurations (ssh/rsh) are identical.
If yes, it'd be enough just to check TST_NEEDS_CMDS also on rhost.
I'm for this variant as it's a simpler change. I'll send a patch tomorrow.
Kind regards,
Petr
^ permalink raw reply [flat|nested] 8+ messages in thread
* [LTP] [PATCH 1/1] net/sendfile01.sh: Check with timeout
2020-04-23 22:12 ` Petr Vorel
@ 2020-04-24 12:11 ` Petr Vorel
2020-04-25 3:28 ` Li Wang
0 siblings, 1 reply; 8+ messages in thread
From: Petr Vorel @ 2020-04-24 12:11 UTC (permalink / raw)
To: ltp
Hi Alexey,
> > > I guess nothing controversial here as failure of starting server is
> > > guarded by -s.
> > > I was thinking about using TST_RETRY_FUNC, but passing command to it
> > > leads to: tst_rhost_run: unknown option: l
> > Hi Petr,
> > eval might help in this case, take a look at tst_retry() in test.sh
> Good point.
> > old api, not sure why exactly it was removed in the new one...
> It was designed from scratch I guess.
> But this patch makes sense to me, I'll test it tomorrow.
Actually, I now consider a bit cleaner and safer solution to *not* use eval
and require test to specify function. E.g.:
+retry_fnc()
+{
+ tst_rhost_run -c 'ss -ltp' | grep -q "$port.*testsf"
+}
+
do_setup()
{
@@ -28,7 +33,7 @@ do_setup()
tst_rhost_run -s -b -c "$server $(tst_ipaddr rhost) $port"
server_started=1
tst_res TINFO "wait for the server to start"
- sleep 1
+ TST_RETRY_FUNC retry_fnc 0
}
Instead of simple:
do_setup()
@@ -28,7 +28,7 @@ do_setup()
tst_rhost_run -s -b -c "$server $(tst_ipaddr rhost) $port"
server_started=1
tst_res TINFO "wait for the server to start"
- sleep 1
+ TST_RETRY_FUNC "tst_rhost_run -c 'ss -ltp' | grep -q '$port.*testsf'" 0
}
But I don't have strong opinion on it.
Cyril, Li, any preference?
Kind regards,
Petr
> > index 1d8a71d9f..e34edb26a 100644
> > --- a/testcases/lib/tst_test.sh
> > +++ b/testcases/lib/tst_test.sh
> > @@ -207,7 +207,7 @@ TST_RETRY_FN_EXP_BACKOFF()
> > fi
> > while true; do
> > - $tst_fun
> > + eval "$tst_fun"
> > if [ "$?" = "$tst_exp" ]; then
> > break
> > fi
...
^ permalink raw reply [flat|nested] 8+ messages in thread
* [LTP] [PATCH 1/1] net/sendfile01.sh: Check with timeout
2020-04-23 19:04 ` Alexey Kodanev
2020-04-23 22:12 ` Petr Vorel
@ 2020-04-24 12:53 ` Petr Vorel
2020-04-24 13:06 ` Petr Vorel
1 sibling, 1 reply; 8+ messages in thread
From: Petr Vorel @ 2020-04-24 12:53 UTC (permalink / raw)
To: ltp
Hi all,
> > What bothers me more, that TST_NEEDS_CMDS does not check command on
> > rhost. Do we want to have something like TST_NEEDS_CMDS_RHOST or we just
> > don't care?
> In general, yes, we need to check if a command exists on the remote host.
> Yet another variable, what about checking what in TST_NEEDS_CMDS on the
> remote host? Though TST_NEEDS_CMDS_RHOST looks quite well.
Hm, looking on the implementation. tst_require_cmds() and tst_cmd_available()
are functions, thus not runnable with tst_rhost_run(). We can add this code to
custom shell script which checks or C code (shell has dependencies). Or any
other idea?
Kind regards,
Petr
^ permalink raw reply [flat|nested] 8+ messages in thread
* [LTP] [PATCH 1/1] net/sendfile01.sh: Check with timeout
2020-04-24 12:53 ` Petr Vorel
@ 2020-04-24 13:06 ` Petr Vorel
0 siblings, 0 replies; 8+ messages in thread
From: Petr Vorel @ 2020-04-24 13:06 UTC (permalink / raw)
To: ltp
Hi all,
> > > What bothers me more, that TST_NEEDS_CMDS does not check command on
> > > rhost. Do we want to have something like TST_NEEDS_CMDS_RHOST or we just
> > > don't care?
> > In general, yes, we need to check if a command exists on the remote host.
> > Yet another variable, what about checking what in TST_NEEDS_CMDS on the
> > remote host? Though TST_NEEDS_CMDS_RHOST looks quite well.
> Hm, looking on the implementation. tst_require_cmds() and tst_cmd_available()
> are functions, thus not runnable with tst_rhost_run(). We can add this code to
> custom shell script which checks or C code (shell has dependencies). Or any
> other idea?
BTW normally it works to load script and then run function.
$ cat foo.sh
#!/bin/sh
foo() { echo "foo: $$"; }
$ sh -c ". foo.sh; foo"
foo: 32479
But this somehow doesn't work for tst_net.sh:
diff --git testcases/lib/tst_net.sh testcases/lib/tst_net.sh
index 1ec03c738..728987f0b 100644
--- testcases/lib/tst_net.sh
+++ testcases/lib/tst_net.sh
@@ -57,6 +57,7 @@ tst_net_remote_tmpdir()
tst_net_setup()
{
+ tst_rhost_run -c ". tst_net.sh; tst_require_cmds $TST_NEEDS_CMDS"
tst_net_remote_tmpdir
[ -n "$TST_SETUP_CALLER" ] && $TST_SETUP_CALLER
results in:
/opt/ltp/testcases/bin/tst_net.sh: line 944: tst_cmd_available: command not found
tst_cmd_available() is needed for the end of tst_net.sh:
if [ -z "$TST_USE_LEGACY_API" ] && ! tst_cmd_available ping6; then
ping6()
{
ping -6 $@
}
if [ -z "$_tst_net_ping6_warn_printed" ]; then
tst_res_ TINFO "ping6 binary/symlink is missing, using workaround. Please, report missing ping6 to your distribution."
export _tst_net_ping6_warn_printed=1
fi
fi
=> sourcing of shell scripts with "." does not work on 'sh -c'.
Kind regards,
Petr
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [LTP] [PATCH 1/1] net/sendfile01.sh: Check with timeout
2020-04-24 12:11 ` Petr Vorel
@ 2020-04-25 3:28 ` Li Wang
2020-04-26 8:05 ` Petr Vorel
0 siblings, 1 reply; 8+ messages in thread
From: Li Wang @ 2020-04-25 3:28 UTC (permalink / raw)
To: ltp
Petr Vorel <pvorel@suse.cz> wrote:
...
>
> > > old api, not sure why exactly it was removed in the new one...
> > It was designed from scratch I guess.
> > But this patch makes sense to me, I'll test it tomorrow.
>
> Actually, I now consider a bit cleaner and safer solution to *not* use eval
> and require test to specify function. E.g.:
>
Why not use eval for that? It helps us to perform more commands directly
without wrap into function.
> +retry_fnc()
> +{
> + tst_rhost_run -c 'ss -ltp' | grep -q "$port.*testsf"
> +}
> +
> do_setup()
> {
>
> @@ -28,7 +33,7 @@ do_setup()
> tst_rhost_run -s -b -c "$server $(tst_ipaddr rhost) $port"
> server_started=1
> tst_res TINFO "wait for the server to start"
> - sleep 1
> + TST_RETRY_FUNC retry_fnc 0
> }
>
> Instead of simple:
>
> do_setup()
> @@ -28,7 +28,7 @@ do_setup()
> tst_rhost_run -s -b -c "$server $(tst_ipaddr rhost) $port"
> server_started=1
> tst_res TINFO "wait for the server to start"
> - sleep 1
> + TST_RETRY_FUNC "tst_rhost_run -c 'ss -ltp' | grep -q
> '$port.*testsf'" 0
> }
>
> But I don't have strong opinion on it.
> Cyril, Li, any preference?
>
If no more strict reasons I prefer to go the simpler way. And there is no
need to wrap a retry_fun() I think.
--
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200425/a8bc5114/attachment.htm>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [LTP] [PATCH 1/1] net/sendfile01.sh: Check with timeout
2020-04-25 3:28 ` Li Wang
@ 2020-04-26 8:05 ` Petr Vorel
0 siblings, 0 replies; 8+ messages in thread
From: Petr Vorel @ 2020-04-26 8:05 UTC (permalink / raw)
To: ltp
Hi Li,
> > > > old api, not sure why exactly it was removed in the new one...
> > > It was designed from scratch I guess.
> > > But this patch makes sense to me, I'll test it tomorrow.
> > Actually, I now consider a bit cleaner and safer solution to *not* use eval
> > and require test to specify function. E.g.:
> Why not use eval for that? It helps us to perform more commands directly
> without wrap into function.
> > +retry_fnc()
> > +{
> > + tst_rhost_run -c 'ss -ltp' | grep -q "$port.*testsf"
> > +}
> > +
> > do_setup()
> > {
> > @@ -28,7 +33,7 @@ do_setup()
> > tst_rhost_run -s -b -c "$server $(tst_ipaddr rhost) $port"
> > server_started=1
> > tst_res TINFO "wait for the server to start"
> > - sleep 1
> > + TST_RETRY_FUNC retry_fnc 0
> > }
> > Instead of simple:
> > do_setup()
> > @@ -28,7 +28,7 @@ do_setup()
> > tst_rhost_run -s -b -c "$server $(tst_ipaddr rhost) $port"
> > server_started=1
> > tst_res TINFO "wait for the server to start"
> > - sleep 1
> > + TST_RETRY_FUNC "tst_rhost_run -c 'ss -ltp' | grep -q
> > '$port.*testsf'" 0
> > }
> > But I don't have strong opinion on it.
> > Cyril, Li, any preference?
> If no more strict reasons I prefer to go the simpler way. And there is no
> need to wrap a retry_fun() I think.
I'm a bit careful and try to avoid eval in scripts for security reasons.
But ok, LTP code is a bit different from running shell scripts on the server,
security does not matters on SUT, so I'm not against it. I just wanted to hear
more opinions on that, thanks for your comment.
Kind regards,
Petr
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-04-26 8:05 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-21 18:00 [LTP] [PATCH 1/1] net/sendfile01.sh: Check with timeout Petr Vorel
2020-04-23 19:04 ` Alexey Kodanev
2020-04-23 22:12 ` Petr Vorel
2020-04-24 12:11 ` Petr Vorel
2020-04-25 3:28 ` Li Wang
2020-04-26 8:05 ` Petr Vorel
2020-04-24 12:53 ` Petr Vorel
2020-04-24 13:06 ` Petr Vorel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.