Thu, Aug 16, 2012 at 02:10:57PM CEST, jtluka(a)redhat.com wrote:
Hi, I'm seeing some oddness in this patch. See inline ...
Thu, Aug 16, 2012 at 01:43:48PM CEST, jirka(a)fedoraproject.org wrote:
>commit 46e03e6884b0817a962b414beff25c1eb2e1af5f
>Author: Jiri Pirko <jiri(a)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(a)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(a)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
I'm not :) In this case I only use it to block until kmod is in use.
I do not case about the result, because "modprobe -r" which is called
right next would fail anyway.
>+ 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(a)lists.fedorahosted.org
>https://lists.fedorahosted.org/mailman/listinfo/lnst-developers
_______________________________________________
LNST-developers mailing list
LNST-developers(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/lnst-developers