Re: [PATCH 2/3] NetTest{Slave, Controller}: add is_installed tcpdump check

[Jan Tluka]

Fri, Jul 22, 2016 at 01:09:58PM CEST, [e-mail] wrote:
> From: Ondrej Lichtner
>
> When the --packet-capture argument was specified and tcpdump wasn't
> installed on one of the machines LNST would fail and report a very
> nondescriptive exception.
>
> This commit adds a is_installed check to the slave method
> "start_packet_capture" and the generated exception is then handled on
> the Controller, reporting a more useful error message.
>
> Fixes #161.
> Signed-off-by: Ondrej Lichtner
>
> lnst/Controller/NetTestController.py | 11 +++++++++--
> lnst/Slave/NetTestSlave.py           |  4 ++++
> 2 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/lnst/Controller/NetTestController.py b/lnst/Controller/NetTestController.py
> index 889dfa7..791dd95 100644
> --- a/lnst/Controller/NetTestController.py
> +++ b/lnst/Controller/NetTestController.py
> @@ -651,8 +651,15 @@ class NetTestController:
>             self._cleanup_slaves()
>             raise
>    if self._packet_capture:
>        self._start_packet_capture()
>    try:
>        if self._packet_capture:

Just wondering if moving if-clause out of try-except block would not
look better. Besides that ack.

Acked-by: Jan Tluka
>            self._start_packet_capture()
>    except Exception as exc:
>        logging.error("Couldn't start packet capture.")
>        logging.error(str(exc))
>        self._cleanup_slaves()
>        return {"passed": False,
>                "err_msg": str(exc)}
>
>   err = None
>   try:
>
> diff --git a/lnst/Slave/NetTestSlave.py b/lnst/Slave/NetTestSlave.py
> index 8d8fada..b18f47d 100644
> --- a/lnst/Slave/NetTestSlave.py
> +++ b/lnst/Slave/NetTestSlave.py
> @@ -31,6 +31,7 @@ from lnst.Common.NetTestCommand import NetTestCommandContext
> from lnst.Common.NetTestCommand import NetTestCommand
> from lnst.Common.NetTestCommand import DEFAULT_TIMEOUT
> from lnst.Common.Utils import check_process_running
> +from lnst.Common.Utils import is_installed
> from lnst.Common.ConnectionHandler import send_data
> from lnst.Common.ConnectionHandler import ConnectionHandler
> from lnst.Common.Config import lnst_config
> @@ -359,6 +360,9 @@ class SlaveMethods:
>         return True
> def start_packet_capture(self, filt):
>    if not is_installed("tcpdump"):
>        raise Exception("Can't start packet capture, tcpdump not available")
>   files = {}
>   for if_id, dev in self._if_manager.get_mapped_devices().iteritems():
>       if dev.get_netns() != None:
>
> -- 
> 2.9.0
> _______________________________________________
> LNST-developers mailing list
> [e-mail]
> https://lists.fedorahosted.org/admin/lists/[e-mail]...