openstack/neutron
Code Issues Proposed changes

Call DVR VMARP notify outside of transaction

Browse Source 
The dvr vmarp table update notification was being called inside
of the delete_port transaction in ML2, which can cause a yield
and lead to the glorious mysql/eventlet deadlock.

This patch moves it outside the transaction and adjusts it to
use an existing port dictionary rather than re-looking it up since
the port is now gone from the DB by the time it is called.

Closes-Bug: #1377241
Change-Id: I0b4dac61e49b2a926353f8478e421cd1a70be038
This commit is contained in:
Kevin Benton 2014-10-11 03:42:47 -07:00
parent 53b0e04ba1
commit 4c2b42e217
4 changed files with 28 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
neutron/db/l3_dvr_db.py
View File

@ -538,13 +538,12 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin,
self._populate_subnet_for_ports(context, port_list)
return port_list
def dvr_vmarp_table_update(self, context, port_id, action):
def dvr_vmarp_table_update(self, context, port_dict, action):
"""Notify the L3 agent of VM ARP table changes.
Provide the details of the VM ARP to the L3 agent when
a Nova instance gets created or deleted.
"""
port_dict = self._core_plugin._get_port(context, port_id)
# Check this is a valid VM port
if ("compute:" not in port_dict['device_owner'] or
not port_dict['fixed_ips']):

4
neutron/plugins/ml2/plugin.py
View File

@ -1009,8 +1009,6 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
if l3plugin:
router_ids = l3plugin.disassociate_floatingips(
context, id, do_notify=False)
if is_dvr_enabled:
l3plugin.dvr_vmarp_table_update(context, id, "del")
LOG.debug("Calling delete_port for %(port_id)s owned by %(owner)s"
% {"port_id": id, "owner": device_owner})
@ -1018,6 +1016,8 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
# now that we've left db transaction, we are safe to notify
if l3plugin:
if is_dvr_enabled:
l3plugin.dvr_vmarp_table_update(context, port, "del")
l3plugin.notify_routers_updated(context, router_ids)
for router in removed_routers:
try:

3
neutron/plugins/ml2/rpc.py
View File

@ -165,7 +165,8 @@ class RpcCallbacks(n_rpc.RpcCallback,
utils.is_extension_supported(l3plugin,
q_const.L3_DISTRIBUTED_EXT_ALIAS)):
try:
l3plugin.dvr_vmarp_table_update(rpc_context, port_id, "add")
port = plugin._get_port(rpc_context, port_id)
l3plugin.dvr_vmarp_table_update(rpc_context, port, "add")
except exceptions.PortNotFound:
LOG.debug('Port %s not found during ARP update', port_id)

25
neutron/tests/unit/ml2/test_ml2_plugin.py
View File

@ -988,9 +988,9 @@ class TestFaultyMechansimDriver(Ml2PluginV2FaultyDriverTestCase):
self._delete('ports', port['port']['id'])
class TestMl2PluginCreateUpdatePort(base.BaseTestCase):
class TestMl2PluginCreateUpdateDeletePort(base.BaseTestCase):
def setUp(self):
super(TestMl2PluginCreateUpdatePort, self).setUp()
super(TestMl2PluginCreateUpdateDeletePort, self).setUp()
self.context = mock.MagicMock()
def _ensure_transaction_is_closed(self):
@ -1043,3 +1043,24 @@ class TestMl2PluginCreateUpdatePort(base.BaseTestCase):
plugin._notify_l3_agent_new_port.assert_called_once_with(
self.context, new_host_port)
def test_vmarp_table_update_outside_of_delete_transaction(self):
l3plugin = mock.Mock()
l3plugin.dvr_vmarp_table_update = (
lambda *args, **kwargs: self._ensure_transaction_is_closed())
l3plugin.dvr_deletens_if_no_port.return_value = []
l3plugin.supported_extension_aliases = [
'router', constants.L3_AGENT_SCHEDULER_EXT_ALIAS,
constants.L3_DISTRIBUTED_EXT_ALIAS
]
with contextlib.nested(
mock.patch.object(ml2_plugin.Ml2Plugin, '__init__',
return_value=None),
mock.patch.object(manager.NeutronManager,
'get_service_plugins',
return_value={'L3_ROUTER_NAT': l3plugin}),
):
plugin = self._create_plugin_for_create_update_port(mock.Mock())
# deleting the port will call dvr_vmarp_table_update, which will
# run the transaction balancing function defined in this test
plugin.delete_port(self.context, 'fake_id')