[LTP] [PATCH 2/2] testcases/lib: tst_net.sh: Do not use stderr in tst_rhost_run()

[Petr Vorel]

> Hi!
> > I'm not sure about this. Maybe it's safe, because stderr gets to the output.
> > But in case some network test need to verify something which is defined on
> > stderr it will fails (when tst_rhost_run is run with -s).

> That's why I send the first patch that I know is 100% safe.

+1

> > FYI *without* this patch:

> > 	# route-redirect.sh
> > 	...
> > 	route-redirect 1 TINFO: timeout per run is 0h 5m 0s
> > 	route-redirect 1 TBROK: 'ns-icmp_redirector -I ltp_ns_veth1 -b' failed on '': 'sh: 1: ns-icmp_redirector: not found'
> > 	route-redirect 1 TWARN: 'killall -SIGHUP ns-icmp_redirector' failed on '': 'ns-icmp_redirector: no process found'
> > 	ns-icmp_redirector: no process found

> > and *with* this patch:

> > 	# route-redirect.sh
> > 	...
> > 	route-redirect 1 TINFO: timeout per run is 0h 5m 0s
> > 	sh: 1: ns-icmp_redirector: not found
> > 	route-redirect 1 TBROK: 'ns-icmp_redirector -I ltp_ns_veth1 -b' failed on '': ''
> > 	ns-icmp_redirector: no process found
> > 	route-redirect 1 TWARN: 'killall -SIGHUP ns-icmp_redirector' failed on '': ''

> > Info is there, but not on the same line (second '').

> > Also with the change below it would be more meaningful (fixing first empty ''):

> > route-redirect 1 TBROK: 'ns-icmp_redirector -I ltp_ns_veth1 -b' failed on NETNS: 'sh: 1: ns-icmp_redirector: not found'
> > route-redirect 1 TWARN: 'killall -SIGHUP ns-icmp_redirector' failed on NETNS: 'ns-icmp_redirector: no process found'
> > ns-icmp_redirector: no process found

> > I also wrote conditional quiet on stderr (not sent to ML), but actually if we
> > merge this and need sometimes to parse stderr, the opposite would be needed.

> I supose that we would need to explicitly tell the tst_rhost_run() if we
> want stderr included or not. That's probably something to be done after
> the release.

Generally stderr was always needed. It got broken when tst_ns_exec, which runs
rhost commands on network namespaces, e.g. not the command it runs tst_rhost_run
with -c parameter, but how exactly it is executed. ATM I'm aware only about
tst_rhost_run runs in tst_net_setup_network(), which got broken due eval.

I can send a patch, but IMHO we should concentrate (after the release) on
rewriting as much as we can from tst_net.sh to single C binary + help to
simplify the architecture.

Kind regards,
Petr

init_ltp_netspace()
{
	export LTP_NETNS="${LTP_NETNS:-tst_ns_exec $pid net,mnt}"
	...

tst_rhost_run()
{
	...
	while getopts :bc:su: opt; do
		case "$opt" in
		b) [ "${TST_USE_NETNS:-}" ] && pre_cmd= || pre_cmd="nohup"
		   post_cmd=" > /dev/null 2>&1 &"
		   out="1> /dev/null"
		;;
		c) cmd="$OPTARG" ;;
		s) safe=1 ;;
		u) user="$OPTARG" ;;
		*) tst_brk_ TBROK "tst_rhost_run: unknown option: $OPTARG" ;;
		esac
	done
	...
	sh_cmd="$pre_cmd $cmd $post_cmd"

	if [ -n "${TST_USE_NETNS:-}" ]; then
		use="NETNS"
		rcmd="$LTP_NETNS sh -c"
	else
		tst_require_cmds ssh
		use="SSH"
		rcmd="ssh -nq $user@$RHOST"
	fi
	...
	output=$($rcmd "$sh_cmd" $out 2>&1 || echo 'RTERR')

tst_net_setup_network()
{
...
	eval $(tst_rhost_run -c 'tst_net_iface_prefix -r '$IPV4_RHOST \
		|| echo "exit $?")
...
		eval $(tst_net_iface_prefix $IPV6_LHOST || echo "exit $?")
		eval $(tst_rhost_run -c 'tst_net_iface_prefix -r '$IPV6_RHOST \