]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Fix get_vif_port_by_id to only return relevant ports
authorVincent Untz <vuntz@suse.com>
Sun, 23 Feb 2014 16:57:05 +0000 (17:57 +0100)
committerThomas Goirand <thomas@goirand.fr>
Thu, 13 Mar 2014 07:20:34 +0000 (15:20 +0800)
This is returning any port, even if it's not on the switch that we're
looking at. As a side-effect, this means that we can actually manipulate
these ports while we really shouldn't.

Co-Authored-By: Rossella Sblendido <rsblendido@suse.com>
Change-Id: Ia4f4e93237c1c2ea6cb4b6c2f5adf78d6b34c7bf
Closes-Bug: #1283765

neutron/agent/linux/ovs_lib.py
neutron/tests/unit/openvswitch/test_ovs_lib.py

index 8a1e6dd088cb85b5263e05ebadc61e6005ba6682..53eceaeb42e4cd5ff19673e3e378663c8d3ab954 100644 (file)
@@ -396,6 +396,13 @@ class OVSBridge(BaseOVS):
             # rows since the interface identifier is unique
             data = json_result['data'][0]
             port_name = data[name_idx]
+            switch = get_bridge_for_iface(self.root_helper, port_name)
+            if switch != self.br_name:
+                LOG.info(_("Port: %(port_name)s is on %(switch)s,"
+                           " not on %(br_name)s"), {'port_name': port_name,
+                                                    'switch': switch,
+                                                    'br_name': self.br_name})
+                return
             ofport = data[ofport_idx]
             # ofport must be integer otherwise return None
             if not isinstance(ofport, int) or ofport == -1:
index 0035a7f34ac5ee9670464b08e7f1b724b6c68e58..27b17060748cb33dde34444e4eaf590b9cce5376 100644 (file)
@@ -605,7 +605,7 @@ class OVS_Lib_Test(base.BaseTestCase):
             with testtools.ExpectedException(Exception):
                 self.br.get_local_port_mac()
 
-    def _test_get_vif_port_by_id(self, iface_id, data):
+    def _test_get_vif_port_by_id(self, iface_id, data, br_name=None):
         headings = ['external_ids', 'name', 'ofport']
         # Each element is a tuple of (expected mock call, return_value)
         expected_calls_and_values = [
@@ -615,7 +615,15 @@ class OVS_Lib_Test(base.BaseTestCase):
                         'external_ids:iface-id="%s"' % iface_id],
                        root_helper=self.root_helper),
              self._encode_ovs_json(headings, data))]
+        if data:
+            if not br_name:
+                br_name = self.BR_NAME
 
+            expected_calls_and_values.append(
+                (mock.call(["ovs-vsctl", self.TO,
+                            "iface-to-br", data[0][headings.index('name')]],
+                           root_helper=self.root_helper),
+                 br_name))
         tools.setup_mock_calls(self.execute, expected_calls_and_values)
         vif_port = self.br.get_vif_port_by_id(iface_id)
 
@@ -654,3 +662,10 @@ class OVS_Lib_Test(base.BaseTestCase):
 
     def test_get_vif_by_port_id_with_no_data(self):
         self.assertIsNone(self._test_get_vif_port_by_id('whatever', []))
+
+    def test_get_vif_by_port_id_different_bridge(self):
+        external_ids = [["iface-id", "tap99id"],
+                        ["iface-status", "active"]]
+        data = [[["map", external_ids], "tap99", 1]]
+        self.assertIsNone(self._test_get_vif_port_by_id('tap99id', data,
+                                                        "br-ext"))