[lnst] ensure correct team cleanup

Jan Tluka jtluka at redhat.com
Thu Aug 16 12:10:57 UTC 2012


Hi, I'm seeing some oddness in this patch. See inline ...

Thu, Aug 16, 2012 at 01:43:48PM CEST, jirka at fedoraproject.org wrote:
>commit 46e03e6884b0817a962b414beff25c1eb2e1af5f
>Author: Jiri Pirko <jiri at resnulli.us>
>Date:   Thu Aug 16 13:35:01 2012 +0200
>
>    ensure correct team cleanup
>    
>    Killall teamd eventualy removes all team devices from the system.
>    However it may happen after lnst does netdev rescan and therefore lnst
>    may still see it. Resolve this by waiting for all kmod users to
>    disappear and, just to be save, remove all team kmods.
>    
>    Signed-off-by: Jiri Pirko <jiri at resnulli.us>
>
> Common/Utils.py              |   18 ++++++++++++++++++
> NetConfig/NetConfigDevice.py |   20 ++++++++------------
> 2 files changed, 26 insertions(+), 12 deletions(-)
>---
>diff --git a/Common/Utils.py b/Common/Utils.py
>index f6ba201..5f3f251 100644
>--- a/Common/Utils.py
>+++ b/Common/Utils.py
>@@ -11,6 +11,7 @@ jzupka at redhat.com (Jiri Zupka)
> """
> import logging
> import time
>+import re
> 
> def die_when_parent_die():
>     try:
>@@ -52,3 +53,20 @@ def wait_for(func, timeout, first=0.0, step=1.0, text=None):
> 
>     logging.debug("Timeout elapsed")
>     return None
>+
>+def kmod_in_use(modulename, tries = 1):
>+    tries -= 1
>+    ret = False
>+    mod_file = "/proc/modules"
>+    handle = open(mod_file, "r")
>+    for line in handle:
>+        match = re.match(r'^(\S+)\s\d+\s(\d+).*$', line)
>+        if not match or not match.groups()[0] in re.split('\s+', modulename):
>+            continue
>+        if int(match.groups()[1]) != 0:
>+            ret = True
>+        break
>+    handle.close()
>+    if (ret and tries):
>+        return kmod_in_use(modulename, tries)
>+    return ret
>diff --git a/NetConfig/NetConfigDevice.py b/NetConfig/NetConfigDevice.py
>index 1e8a9aa..534e914 100644
>--- a/NetConfig/NetConfigDevice.py
>+++ b/NetConfig/NetConfigDevice.py
>@@ -16,6 +16,7 @@ import re
> import sys
> from Common.ExecCmd import exec_cmd
> from NetConfigCommon import get_slaves, get_option
>+from Common.Utils import kmod_in_use
> 
> class NetConfigDeviceGeneric:
>     '''
>@@ -23,6 +24,7 @@ class NetConfigDeviceGeneric:
>     extend this one.
>     '''
>     _modulename = ""
>+    _moduleload = True
>     _moduleparams = ""
>     _cleanupcmd = ""
> 
>@@ -58,24 +60,16 @@ class NetConfigDeviceGeneric:
> 
>     @classmethod
>     def type_init(self):
>-        if self._modulename:
>+        if self._modulename and self._moduleload:
>             exec_cmd("modprobe %s %s" % (self._modulename, self._moduleparams))
> 
>     @classmethod
>-    def _module_check(self):
>-        if self._modulename:
>-            output = exec_cmd("modinfo -F filename %s" % self._modulename, die_on_err=False)[0]
>-            for line in output.split("\n"):
>-                if re.match(r'^.*\/%s\.ko$' % self._modulename, line):
>-                    return True
>-        False
>-
>-    @classmethod
>     def type_cleanup(self):
>-        if self._modulename and self._module_check():
>-            exec_cmd("modprobe -r %s" % self._modulename, die_on_err=False)
>         if self._cleanupcmd:
>             exec_cmd(self._cleanupcmd, die_on_err=False)
>+        if self._modulename:
>+            kmod_in_use(self._modulename, 300)

... kmod_in_use returns True or False, so I think we're missing some
condition check, like:
              if not kmod_in_use(...):
                  remove_module

>+            exec_cmd("modprobe -r %s" % self._modulename, die_on_err=False)
> 
> class NetConfigDeviceEth(NetConfigDeviceGeneric):
>     def configure(self):
>@@ -217,6 +211,8 @@ class NetConfigDeviceVlan(NetConfigDeviceGeneric):
> 
> class NetConfigDeviceTeam(NetConfigDeviceGeneric):
>     _pidfile = None
>+    _modulename = "team_mode_roundrobin team_mode_activebackup team_mode_broadcast team_mode_loadbalance team"
>+    _moduleload = False
>     _cleanupcmd = "killall -q teamd"
> 
>     def configure(self):
>_______________________________________________
>LNST-developers mailing list
>LNST-developers at lists.fedorahosted.org
>https://lists.fedorahosted.org/mailman/listinfo/lnst-developers


More information about the LNST-developers mailing list