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

[Jan Tluka]

Fri, Jul 22, 2016 at 01:09:58PM CEST, olichtne(a)redhat.com wrote:
> From: Ondrej Lichtner <olichtne(a)redhat.com&gt;
>
> 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 <olichtne(a)redhat.com&gt;
> ---
>  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 <jtluka(a)redhat.com&gt;

>+                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
&gt;lnst-developers(a)lists.fedorahosted.org
>https://lists.fedorahosted.org/admin/lists/[e-mail]