Change in vdsm[master]: Multiple Gateways[4/2]: Fixed ifdown bug for DHCP interfaces

amuller at redhat.com amuller at redhat.com
Mon Jul 8 08:44:52 UTC 2013


Assaf Muller has uploaded a new change for review.

Change subject: Multiple Gateways[4/2]: Fixed ifdown bug for DHCP interfaces
......................................................................

Multiple Gateways[4/2]: Fixed ifdown bug for DHCP interfaces

If the admin manually ifdown'd an interface of ours following
patch 3 in the series, we failed to remove the routes and rules
we created during ifup. The reason is that since patch 3 we
are working with files. The dhclient hook created a file during
ifdown and finished the ifdown operation before VDSM got the
notification that the file was created. By the time VDSM got
the notification the interface was already down and thus could
not find any information about the interface.

This patch fixes that bug by adding the device to the rule
while adding the rule, so that we may use the device name
to look up all rules created for that device.

NOTE: The change to the ip rules need re-verification of
the feature. I'll use this opportunity to test the feature
from top to bottom using the engine for input.

Change-Id: If1ed9efc548ec0342db9ea0ae4692d2c0aec93df
Signed-off-by: Assaf Muller <amuller at redhat.com>
---
M lib/vdsm/ipwrapper.py
M vdsm/sourceRoute.py
2 files changed, 28 insertions(+), 31 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/52/16552/1

diff --git a/lib/vdsm/ipwrapper.py b/lib/vdsm/ipwrapper.py
index 7aafbc3..326101e 100644
--- a/lib/vdsm/ipwrapper.py
+++ b/lib/vdsm/ipwrapper.py
@@ -108,7 +108,7 @@
 
 
 class Rule(object):
-    def __init__(self, table, source=None, destination=None):
+    def __init__(self, table, source=None, destination=None, inDevice=None):
         if source:
             if not (_isValid(source, IPAddress) or
                     _isValid(source, IPNetwork)):
@@ -124,6 +124,7 @@
         self.table = table
         self.source = source
         self.destination = destination
+        self.inDevice = inDevice
 
     @classmethod
     def fromText(cls, text):
@@ -161,8 +162,10 @@
             source = None
         if destination == 'all':
             destination = None
+        inDevice = data.get('dev') or data.get('iif')
 
-        return cls(table, source=source, destination=destination)
+        return cls(table, source=source, destination=destination,
+                   inDevice=inDevice)
 
     def __str__(self):
         str = 'from '
@@ -172,6 +175,8 @@
             str += 'all'
         if self.destination:
             str += ' to %s' % self.destination
+        if self.inDevice:
+            str += ' dev %s' % self.inDevice
 
         str += ' table %s' % self.table
 
@@ -221,12 +226,6 @@
     command = [_IP_BINARY.cmd, 'route', 'del']
     command += route
     _execCmd(command)
-
-
-def routeLinkNetForDevice(device):
-    command = [_IP_BINARY.cmd, 'route', 'show', 'dev', device, 'scope', 'link',
-               'proto', 'kernel']
-    return _execCmd(command)
 
 
 def ruleList():
diff --git a/vdsm/sourceRoute.py b/vdsm/sourceRoute.py
index 7de484f..8e3fd5f 100644
--- a/vdsm/sourceRoute.py
+++ b/vdsm/sourceRoute.py
@@ -24,7 +24,6 @@
 
 from vdsm import netinfo
 from vdsm.ipwrapper import Route
-from vdsm.ipwrapper import routeLinkNetForDevice
 from vdsm.ipwrapper import routeShowTable
 from vdsm.ipwrapper import Rule
 from vdsm.ipwrapper import ruleList
@@ -54,7 +53,8 @@
 
     def _buildRules(self):
         return [Rule(source=self.network, table=self.table),
-                Rule(destination=self.network, table=self.table)]
+                Rule(destination=self.network, table=self.table,
+                     device=self.device)]
 
     def configure(self, ipaddr, mask, gateway):
         if gateway in (None, '0.0.0.0') or not ipaddr or not mask:
@@ -142,35 +142,33 @@
             return None
 
     @staticmethod
-    def _getRules(network):
+    def _getRules(device):
         allRules = [Rule.fromText(entry) for entry in ruleList()]
-        rules = [rule for rule in allRules if rule.source == network or
-                 rule.destination == network]
+        rules = [rule for rule in allRules if rule.inDevice == device]
 
         if not rules:
-            logging.error("Rules not found for network %s" % network)
+            logging.error("Rules not found for device %s" % device)
+
+        network = DynamicSourceRoute._getNetwork(rules)
+        rules += [rule for rule in allRules if rule.source == network]
 
         return rules
 
     @staticmethod
-    def _getNetwork(device):
-        output = routeLinkNetForDevice(device)
-
-        if output:
-            route = Route.parse(output[0])
-            return route['network']
-        logging.error("Network for given device name not found.")
-        return None
+    def _getNetwork(rules):
+        if rules:
+            return rules[0].destination
+        else:
+            logging.error("Network not found")
+            return None
 
     def remove(self):
         logging.info("Removing gateway - device: %s" % self.device)
 
-        network = self._getNetwork(self.device)
-        if network:
-            rules = self._getRules(network)
-            if rules:
-                table = self._getTable(rules)
-                if table:
-                    self.configurator.removeSourceRoute(
-                        self._getRoutes(table, self.device), rules,
-                        self.device)
+        rules = self._getRules(self.device)
+        if rules:
+            table = self._getTable(rules)
+            if table:
+                self.configurator.removeSourceRoute(
+                    self._getRoutes(table, self.device), rules,
+                    self.device)


-- 
To view, visit http://gerrit.ovirt.org/16552
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: If1ed9efc548ec0342db9ea0ae4692d2c0aec93df
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Assaf Muller <amuller at redhat.com>


More information about the vdsm-patches mailing list