openstack
/
neutron
Code Issues Proposed changes

l3_db: not use L2 plugin _get_port unnecessarily 

This patch is clean up to prevent future breakage by eliminating
potentially dangerous code.

l3_db uses L2 plugin _get_port method unnecessarily instead of get_port.
It's dangerous because _get_port returns ORM db object which allows
the caller to update db rows directly. So the caller of _get_port may
update port db without notifying L2 plugin unintentionally.
In that case, L2 plugin or ML2 mechanism driver will be confused.
This patch replace _get_port with get_port method where possible.

Change-Id: I5a23f6cac5ea359645e6947fd69978f060c4ba97
Related-Bug: #1475093
This commit is contained in:
Isaku Yamahata 2015-07-15 12:34:12 -07:00
parent c584417967
commit 9ba23658a3
2 changed files with 9 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
neutron/db/l3_db.py
View File

@ -818,7 +818,7 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase):
Retrieve information concerning the internal port where Retrieve information concerning the internal port where
the floating IP should be associated to. the floating IP should be associated to.
""" """
internal_port = self._core_plugin._get_port(context, fip['port_id']) internal_port = self._core_plugin.get_port(context, fip['port_id'])
if not internal_port['tenant_id'] == fip['tenant_id']: if not internal_port['tenant_id'] == fip['tenant_id']:
port_id = fip['port_id'] port_id = fip['port_id']
if 'id' in fip: if 'id' in fip:
@ -1077,23 +1077,23 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase):
deletion checks. deletion checks.
""" """
try: try:
port_db = self._core_plugin._get_port(context, port_id) port = self._core_plugin.get_port(context, port_id)
except n_exc.PortNotFound: except n_exc.PortNotFound:
# non-existent ports don't need to be protected from deletion # non-existent ports don't need to be protected from deletion
return return
if port_db['device_owner'] in self.router_device_owners: if port['device_owner'] in self.router_device_owners:
# Raise port in use only if the port has IP addresses # Raise port in use only if the port has IP addresses
# Otherwise it's a stale port that can be removed # Otherwise it's a stale port that can be removed
fixed_ips = port_db['fixed_ips'] fixed_ips = port['fixed_ips']
if fixed_ips: if fixed_ips:
reason = _('has device owner %s') % port_db['device_owner'] reason = _('has device owner %s') % port['device_owner']
raise n_exc.ServicePortInUse(port_id=port_db['id'], raise n_exc.ServicePortInUse(port_id=port['id'],
reason=reason) reason=reason)
else: else:
LOG.debug("Port %(port_id)s has owner %(port_owner)s, but " LOG.debug("Port %(port_id)s has owner %(port_owner)s, but "
"no IP address, so it can be deleted", "no IP address, so it can be deleted",
{'port_id': port_db['id'], {'port_id': port['id'],
'port_owner': port_db['device_owner']}) 'port_owner': port['device_owner']})
def disassociate_floatingips(self, context, port_id): def disassociate_floatingips(self, context, port_id):
"""Disassociate all floating IPs linked to specific port. """Disassociate all floating IPs linked to specific port.

2
neutron/tests/unit/db/test_l3_dvr_db.py
View File

@ -173,7 +173,7 @@ class L3DvrTestCase(testlib_api.SqlTestCase):
with mock.patch.object(manager.NeutronManager, 'get_plugin') as gp: with mock.patch.object(manager.NeutronManager, 'get_plugin') as gp:
plugin = mock.Mock() plugin = mock.Mock()
gp.return_value = plugin gp.return_value = plugin
plugin._get_port.return_value = port plugin.get_port.return_value = port
self.assertRaises(exceptions.ServicePortInUse, self.assertRaises(exceptions.ServicePortInUse,
self.mixin.prevent_l3_port_deletion, self.mixin.prevent_l3_port_deletion,
self.ctx, self.ctx,