Re: [PATCH 4/5] lnst.Slave.InterfaceManager: remove dependency on Netutils.scan_netdevs

[Jan Tluka]

Fri, Oct 11, 2019 at 04:43:29PM CEST, olichtne(a)redhat.com wrote:
> From: Ondrej Lichtner <olichtne(a)redhat.com&gt;
>
> Refactoring the create_device method a little enables us to completely
> remove the dependency of the module on scan_netdevs. The method should
> now only be used in the legacy NmConfigDevice module that should get
> removed soon.
>
> Signed-off-by: Ondrej Lichtner <olichtne(a)redhat.com&gt;
> ---
>  lnst/Slave/InterfaceManager.py | 29 ++++++++++++++++++++---------
>  lnst/Slave/NetTestSlave.py     |  2 --
>  2 files changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/lnst/Slave/InterfaceManager.py b/lnst/Slave/InterfaceManager.py
> index 13be2bc..cc9f2d5 100644
> --- a/lnst/Slave/InterfaceManager.py
> +++ b/lnst/Slave/InterfaceManager.py
> @@ -19,7 +19,6 @@ import logging
>  from collections import deque
>  from lnst.Slave.NetConfigCommon import get_option
>  from lnst.Common.NetUtils import normalize_hwaddr
> -from lnst.Common.NetUtils import scan_netdevs
>  from lnst.Common.ExecCmd import exec_cmd
>  from lnst.Common.ConnectionHandler import recv_data
>  from lnst.Common.DeviceError import DeviceNotFound, DeviceConfigError, DeviceDeleted
> @@ -89,13 +88,16 @@ class InterfaceManager(object):
>              return []
>  
>      def rescan_devices(self):
> +        self.request_netlink_dump()
> +        self.handle_netlink_msgs()
> +
> +    def request_netlink_dump(self):
>          self._nl_socket.put(
>              None, RTM_GETLINK, msg_flags=NLM_F_REQUEST | NLM_F_DUMP
>          )
>          self._nl_socket.put(
>              None, RTM_GETADDR, msg_flags=NLM_F_REQUEST | NLM_F_DUMP
>          )
> -        self.handle_netlink_msgs()
>  
>      def handle_netlink_msgs(self):
>          self.pull_netlink_messages_into_queue()
> @@ -196,14 +198,23 @@ class InterfaceManager(object):
>          device._create()
>          device._bulk_enabled = False
>  
> -        devs = scan_netdevs()
> -        for dev in devs:
> -            if dev["name"] == device.name:
> -                device._init_netlink(dev['netlink_msg'])
> -                self._devices[dev['index']] = device
> -                return device
> +        self.request_netlink_dump()
> +        self.pull_netlink_messages_into_queue()
>  
> -        return None
> +        device_found = False
> +        while len(self._msg_queue):
> +            msg = self._msg_queue.popleft()
> +            if msg.get_attr("IFLA_IFNAME") == device.name:
> +                device_found = True
> +                device._init_netlink(msg)
> +                self._devices[msg['index']] = device
> +            else:
> +                self._handle_netlink_msg(msg)
> +
> +        if device_found:
> +            return device
> +        else:
> +            raise DeviceError("Device creation failed")
>  
>      def replace_dev(self, if_id, dev):
>          del self._devices[if_id]
> diff --git a/lnst/Slave/NetTestSlave.py b/lnst/Slave/NetTestSlave.py
> index 27f5cce..4f5fc9f 100644
> --- a/lnst/Slave/NetTestSlave.py
> +++ b/lnst/Slave/NetTestSlave.py
> @@ -319,8 +319,6 @@ class SlaveMethods:
>  
>      def create_device(self, clsname, args=[], kwargs={}):
>          dev =  self._if_manager.create_device(clsname, args, kwargs)
> -        if dev is None:
> -            raise Exception("Device creation failed") 
Just wondering if the change of the exception class matters here. If the
DeviceError is excepted somewhere upper in the stack, I'd keep it.

>         return {"ifindex": dev.ifindex, "name": dev.name}
> 
>     def start_packet_capture(self, filt):
>-- 
>2.23.0
>_______________________________________________
>LNST-developers mailing list -- lnst-developers(a)lists.fedorahosted.org
>To unsubscribe send an email to lnst-developers-leave(a)lists.fedorahosted.org
>Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
>List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
>List Archives:
https://lists.fedorahosted.org/archives/list/[e-mail]...