Retry delbr on linux_router deconfig

server/site_linux_router.py

Index: server/site_linux_router.py
diff --git a/server/site_linux_router.py b/server/site_linux_router.py
index dfa1ff3c5239d9aff391cddf320577128db37cf0..fc2d26dd511e35f77b5ca00f96eba6a09bc97ead 100644
--- a/server/site_linux_router.py
+++ b/server/site_linux_router.py
@@ -2,7 +2,7 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
-import logging, re
+import logging, re, time
 from autotest_lib.client.common_lib import error
 from autotest_lib.server import site_eap_tls
 
@@ -314,8 +314,19 @@ class LinuxRouter(object):
         # Tear down the bridge.
         self.router.run("%s link set %s down" % (self.cmd_ip, self.bridgeif),
             ignore_status=True)
-        self.router.run("%s delbr %s" % (self.cmd_brctl, self.bridgeif),
-            ignore_status=True)
+
+        # Try a couple times to remove the bridge; hostapd may still be exiting
+        for attempt in range(3):
+            result = self.router.run("%s delbr %s" %
+                                     (self.cmd_brctl, self.bridgeif),
+                                     ignore_status=True)
+            if not result.stderr:
+                break
+            time.sleep(1)
+        else:
+            raise error.TestFail("Unable to delete bridge %s: %s" %
+                                 (self.bridgeif, result.stderr))

[Sam Leffler, 2010/09/01 21:15:07]

would it be better to wait on the hostapd pid and/or do the delbr in a subshell
where hostapd is launched? (hostapd ...; trap 0 delbr ...)&

[Paul Stewart, 2010/09/01 23:00:47]

The "trap" technique just kicks the can down the road.  At some point we really
need to wait for the old hostapd to die so even if the bridge cleans up
automatically.  The bridge needs to be cleared before we fire up another
hostapd.  The "wait" idea is a nice one, but at least in ash, you can't wait for
a pid that you're not a parent of.  It's unlikely we need to do this more than
once, but we need to cover this likelihood.

+
 
         self.hostapd['configured'] = False