Re: [LTP] [PATCH v1] tst_net.sh: Add more tst_require_cmds check

[Petr Vorel <pvorel@suse.cz>]

Hi Wei,

...
> > But even if "ip ns_create ns_exec ns_ifmove" are needed to be checked to fix
> > tst_rhost_run (not yet convinced), why to check them each time tst_rhost_run is
> > called? It should be checked before first tst_rhost_run call to avoid useless
> > repeating.

> tst_rhost_run already be called everywhere so if you need check before call
> tst_rhost_run, means lot of place/cases maybe need review and update. 

No, the check would be on single place at tst_net.sh.

> Furthermore, check each time in tst_rhost_run can make this function more robust, the 
> old code also do tst_require_cmds if go ssh logic in line 211 for example.

Well, repeated check for ssh is not ideal either (how likely is that ssh will
disappear during testing?), but it's a single command which is used here.
And it's definitely less awkward than checking "ip ns_create ns_exec ns_ifmove"
which aren't used there. tst_net.sh needs cleanup, not more unneeded code.

We could document the reason to make this check more obvious, but the code is
wrong, because for netns (the default) running "tst_net_detect_ipv6 rhost"
before init_ltp_netspace does not make sense, because interfaces aren't defined
(probably error code is not detected, I need to recheck that).

It's actually enough for netns to test IPv6 support only on localhost (it will
be the same on rhost), I'll send a patch to fix that.

Kind regards,
Petr

>  205         sh_cmd="$pre_cmd $cmd $post_cmd"
>  206
>  207         if [ -n "${TST_USE_NETNS:-}" ]; then
>  208                 use="NETNS"
>  209                 rcmd="$LTP_NETNS sh -c"
>  210         else
>  211                 tst_require_cmds ssh
>  212                 use="SSH"
>  213                 rcmd="ssh -nq $user@$RHOST"
>  214         fi

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