[LTP] [PATCH v2 1/1] rpc: TCONF when tests aren't compiled + remove kill warning

[LTP] [PATCH v2 1/1] rpc: TCONF when tests aren't compiled + remove kill warning

[Petr Vorel]

Without this test failure was wrongly reported:
rpc_test 1 TFAIL: tirpc_rpcb_getaddr 10.0.0.2 536875000 failed unexpectedly

Also make sure with variable that server has started before trying to
kill it.

Kill server with SIGPIPE, should be enough and SIGKILL
produced warning, which might be confused:
/opt/ltp/testcases/bin/rpc_test.sh: line 61: 3827 Killed  $SERVER $PROGNUMNOSVC

Reviewed-by: Alexey Kodanev <alexey.kodanev@oracle.com>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Hi,

with this implementation (suggested by Alexey), we get TCONF twice. It's
a bit strange, but I'll keep it, because I like that required binary is
printed:
rpc_test 1 TCONF: 'tirpc_rpcb_getaddr' not found
rpc_test 1 TCONF: LTP compiled without TI-RPC support?

Also added proper detection if kill is needed (related to the change in
the commit) and kill with SIGPIPE (unrelated change, I can add it as
separate commit).

Kind regards,
Petr

 testcases/network/rpc/rpc-tirpc/rpc_test.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/testcases/network/rpc/rpc-tirpc/rpc_test.sh b/testcases/network/rpc/rpc-tirpc/rpc_test.sh
index 30cfd2564..dc97213d0 100755
--- a/testcases/network/rpc/rpc-tirpc/rpc_test.sh
+++ b/testcases/network/rpc/rpc-tirpc/rpc_test.sh
@@ -54,12 +54,13 @@ setup()
 	fi
 
 	[ -n "$CLIENT" ] || tst_brk TBROK "client program not set"
+	tst_check_cmds $CLIENT $SERVER || tst_brk TCONF "LTP compiled without TI-RPC support?"
 }
 
 cleanup()
 {
-	if [ ! -z "$SERVER" ]; then
-		pkill -9 $SERVER > /dev/null 2>&1
+	if [ "$SERVER_STARTED" ]; then
+		pkill -13 -x $SERVER
 		$CLEANER $PROGNUMNOSVC
 	fi
 }
@@ -70,6 +71,7 @@ do_test()
 
 	if [ -n "$SERVER" ]; then
 		$SERVER $PROGNUMNOSVC &
+		SERVER_STARTED=1
 
 		for i in $(seq 1 10); do
 			rpcinfo -p localhost | grep -q $PROGNUMNOSVC && break
-- 
2.26.2

[LTP] [PATCH v2 1/1] rpc: TCONF when tests aren't compiled + remove kill warning

[Petr Vorel]

Hi,

> with this implementation (suggested by Alexey), we get TCONF twice. It's
> a bit strange, but I'll keep it, because I like that required binary is
> printed:
> rpc_test 1 TCONF: 'tirpc_rpcb_getaddr' not found
> rpc_test 1 TCONF: LTP compiled without TI-RPC support?
Please any comment for 2x TCONF. It's a bit strange, but just a tiny detail.
I'd like to have this fix in the release.

Kind regards,
Petr

[LTP] [PATCH v2 1/1] rpc: TCONF when tests aren't compiled + remove kill warning

[Xiao Yang]

On 5/14/20 10:47 PM, Petr Vorel wrote:
> Hi,
>
>> with this implementation (suggested by Alexey), we get TCONF twice. It's
>> a bit strange, but I'll keep it, because I like that required binary is
>> printed:
>> rpc_test 1 TCONF: 'tirpc_rpcb_getaddr' not found
>> rpc_test 1 TCONF: LTP compiled without TI-RPC support?
> Please any comment for 2x TCONF. It's a bit strange, but just a tiny detail.
> I'd like to have this fix in the release.

Hi Petr,

Why don't you use tst_cmd_available? like this:

for i in $CLIENT $SERVER; do

 ??? tst_cmd_available $i || tst_brk TCONF "$i not found.? LTP compiled 
without TI-RPC support?"

done

Best Regards,

Xiao Yang

>
> Kind regards,
> Petr
>
>

[LTP] [PATCH v2 1/1] rpc: TCONF when tests aren't compiled + remove kill warning

[Petr Vorel]

> On 5/14/20 10:47 PM, Petr Vorel wrote:
> > Hi,

> > > with this implementation (suggested by Alexey), we get TCONF twice. It's
> > > a bit strange, but I'll keep it, because I like that required binary is
> > > printed:
> > > rpc_test 1 TCONF: 'tirpc_rpcb_getaddr' not found
> > > rpc_test 1 TCONF: LTP compiled without TI-RPC support?
> > Please any comment for 2x TCONF. It's a bit strange, but just a tiny detail.
> > I'd like to have this fix in the release.

> Hi Petr,

Hi Yang,

> Why don't you use tst_cmd_available? like this:

> for i in $CLIENT $SERVER; do

> ??? tst_cmd_available $i || tst_brk TCONF "$i not found.? LTP compiled
> without TI-RPC support?"
Thanks for a review.
Yep, I suggested similar solution in v1
https://patchwork.ozlabs.org/project/ltp/patch/20200512152701.23625-1-pvorel@suse.cz/

I like solution from v2 (suggested by Alexey) as more elegant, just 2xTCONF is a
bit confusing. But I'm hesitating too much about such a tiny detail :).

Kind regards,
Petr

[LTP] [PATCH v2 1/1] rpc: TCONF when tests aren't compiled + remove kill warning

[Cyril Hrubis]

Hi!
> > with this implementation (suggested by Alexey), we get TCONF twice. It's
> > a bit strange, but I'll keep it, because I like that required binary is
> > printed:
> > rpc_test 1 TCONF: 'tirpc_rpcb_getaddr' not found
> > rpc_test 1 TCONF: LTP compiled without TI-RPC support?
> Please any comment for 2x TCONF. It's a bit strange, but just a tiny detail.
> I'd like to have this fix in the release.

I do not really care how many TCONF the test prints as long as the user
gets a reasonable message.

With that the patch looks good, acked.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 1/1] rpc: TCONF when tests aren't compiled + remove kill warning

[Petr Vorel]

Hi Cyril,

> > > with this implementation (suggested by Alexey), we get TCONF twice. It's
> > > a bit strange, but I'll keep it, because I like that required binary is
> > > printed:
> > > rpc_test 1 TCONF: 'tirpc_rpcb_getaddr' not found
> > > rpc_test 1 TCONF: LTP compiled without TI-RPC support?
> > Please any comment for 2x TCONF. It's a bit strange, but just a tiny detail.
> > I'd like to have this fix in the release.

> I do not really care how many TCONF the test prints as long as the user
> gets a reasonable message.
Yep, it's a really tiny detail.

> With that the patch looks good, acked.
Thanks for your review, pushed.

Kind regards,
Petr

[LTP] [PATCH v2 1/1] rpc: TCONF when tests aren't compiled + remove kill warning

[Xiao Yang]

On 2020/5/13 4:14, Petr Vorel wrote:
> Without this test failure was wrongly reported:
> rpc_test 1 TFAIL: tirpc_rpcb_getaddr 10.0.0.2 536875000 failed unexpectedly
Hi Petr,

Do you know which caused this failure?

Best Regards,
Xiao Yang
>
> Also make sure with variable that server has started before trying to
> kill it.
>
> Kill server with SIGPIPE, should be enough and SIGKILL
> produced warning, which might be confused:
> /opt/ltp/testcases/bin/rpc_test.sh: line 61: 3827 Killed  $SERVER $PROGNUMNOSVC
>
> Reviewed-by: Alexey Kodanev<alexey.kodanev@oracle.com>
> Signed-off-by: Petr Vorel<pvorel@suse.cz>
> ---
> Hi,
>
> with this implementation (suggested by Alexey), we get TCONF twice. It's
> a bit strange, but I'll keep it, because I like that required binary is
> printed:
> rpc_test 1 TCONF: 'tirpc_rpcb_getaddr' not found
> rpc_test 1 TCONF: LTP compiled without TI-RPC support?
>
> Also added proper detection if kill is needed (related to the change in
> the commit) and kill with SIGPIPE (unrelated change, I can add it as
> separate commit).
>
> Kind regards,
> Petr
>
>   testcases/network/rpc/rpc-tirpc/rpc_test.sh | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/testcases/network/rpc/rpc-tirpc/rpc_test.sh b/testcases/network/rpc/rpc-tirpc/rpc_test.sh
> index 30cfd2564..dc97213d0 100755
> --- a/testcases/network/rpc/rpc-tirpc/rpc_test.sh
> +++ b/testcases/network/rpc/rpc-tirpc/rpc_test.sh
> @@ -54,12 +54,13 @@ setup()
>   	fi
>
>   	[ -n "$CLIENT" ] || tst_brk TBROK "client program not set"
> +	tst_check_cmds $CLIENT $SERVER || tst_brk TCONF "LTP compiled without TI-RPC support?"
>   }
>
>   cleanup()
>   {
> -	if [ ! -z "$SERVER" ]; then
> -		pkill -9 $SERVER>  /dev/null 2>&1
> +	if [ "$SERVER_STARTED" ]; then
> +		pkill -13 -x $SERVER
>   		$CLEANER $PROGNUMNOSVC
>   	fi
>   }
> @@ -70,6 +71,7 @@ do_test()
>
>   	if [ -n "$SERVER" ]; then
>   		$SERVER $PROGNUMNOSVC&
> +		SERVER_STARTED=1
>
>   		for i in $(seq 1 10); do
>   			rpcinfo -p localhost | grep -q $PROGNUMNOSVC&&  break

[LTP] [PATCH v2 1/1] rpc: TCONF when tests aren't compiled + remove kill warning

[Petr Vorel]

Hi Xiao,

> On 2020/5/13 4:14, Petr Vorel wrote:
> > Without this test failure was wrongly reported:
> > rpc_test 1 TFAIL: tirpc_rpcb_getaddr 10.0.0.2 536875000 failed unexpectedly
> Hi Petr,

> Do you know which caused this failure?
Certain tests compiled with TI-RPC on glibc (not on libtirpc): rpc_pmap_set,
rpc_pmap_unset, rpc_svc_register. NOTE glibc implementation is fading out,
nothing really important.

Before I just disable these tests, I've asked the experts [1] and posted
patchset to show some debug info.

Kind regards,
Petr

[1] https://sourceforge.net/p/libtirpc/mailman/message/37014386/
[2] https://patchwork.ozlabs.org/project/ltp/list/?series=177741&state=*