Add interprocess locks to net interfaces handlers
Manila uses one service network for all backends, in this case handling of all shared net resources should be covered with locks in network modules. Changes: - added decorator 'device_name_synchronized' to remove concurrency issues. - changed arg positions in related methods to satisfy requirement of decorator. - with change of arg position was removed arg 'network_id' in metaclass 'LinuxInterfaceDriver' as redundant. Change-Id: I7468f0a85bbc2752cfd82ae85d7491636ddea24b Closes-Bug: #1346796
This commit is contained in:
parent
50ac9fef33
commit
ba51874398
|
@ -22,6 +22,7 @@ import six
|
|||
from manila import exception
|
||||
from manila.network.linux import ip_lib
|
||||
from manila.network.linux import ovs_lib
|
||||
from manila.openstack.common import lockutils
|
||||
from manila.openstack.common import log as logging
|
||||
from manila import utils
|
||||
|
||||
|
@ -38,6 +39,22 @@ CONF = cfg.CONF
|
|||
CONF.register_opts(OPTS)
|
||||
|
||||
|
||||
def device_name_synchronized(f):
|
||||
"""Wraps methods with interprocess locks by device names."""
|
||||
|
||||
def wrapped_func(self, *args, **kwargs):
|
||||
device_name = "device_name_%s" % args[0]
|
||||
|
||||
@lockutils.synchronized(device_name, external=True,
|
||||
lock_path="service_instance_locks")
|
||||
def source_func(self, *args, **kwargs):
|
||||
return f(self, *args, **kwargs)
|
||||
|
||||
return source_func(self, *args, **kwargs)
|
||||
|
||||
return wrapped_func
|
||||
|
||||
|
||||
@six.add_metaclass(abc.ABCMeta)
|
||||
class LinuxInterfaceDriver(object):
|
||||
|
||||
|
@ -48,6 +65,7 @@ class LinuxInterfaceDriver(object):
|
|||
def __init__(self):
|
||||
self.conf = CONF
|
||||
|
||||
@device_name_synchronized
|
||||
def init_l3(self, device_name, ip_cidrs, namespace=None):
|
||||
"""Set the L3 settings for the interface using data from the port.
|
||||
|
||||
|
@ -82,7 +100,7 @@ class LinuxInterfaceDriver(object):
|
|||
return (self.DEV_NAME_PREFIX + port['id'])[:self.DEV_NAME_LEN]
|
||||
|
||||
@abc.abstractmethod
|
||||
def plug(self, network_id, port_id, device_name, mac_address,
|
||||
def plug(self, device_name, port_id, mac_address,
|
||||
bridge=None, namespace=None, prefix=None):
|
||||
"""Plug in the interface."""
|
||||
|
||||
|
@ -113,7 +131,8 @@ class OVSInterfaceDriver(LinuxInterfaceDriver):
|
|||
'external-ids:attached-mac=%s' % mac_address]
|
||||
utils.execute(*cmd, run_as_root=True)
|
||||
|
||||
def plug(self, port_id, device_name, mac_address,
|
||||
@device_name_synchronized
|
||||
def plug(self, device_name, port_id, mac_address,
|
||||
bridge=None, namespace=None, prefix=None):
|
||||
"""Plug in the interface."""
|
||||
if not bridge:
|
||||
|
@ -139,6 +158,7 @@ class OVSInterfaceDriver(LinuxInterfaceDriver):
|
|||
LOG.warn(_("Device %s already exists"), device_name)
|
||||
ns_dev.link.set_up()
|
||||
|
||||
@device_name_synchronized
|
||||
def unplug(self, device_name, bridge=None, namespace=None, prefix=None):
|
||||
"""Unplug the interface."""
|
||||
if not bridge:
|
||||
|
@ -160,7 +180,8 @@ class BridgeInterfaceDriver(LinuxInterfaceDriver):
|
|||
|
||||
DEV_NAME_PREFIX = 'ns-'
|
||||
|
||||
def plug(self, port_id, device_name, mac_address,
|
||||
@device_name_synchronized
|
||||
def plug(self, device_name, port_id, mac_address,
|
||||
bridge=None, namespace=None, prefix=None):
|
||||
"""Plugin the interface."""
|
||||
ip = ip_lib.IPWrapper()
|
||||
|
@ -184,6 +205,7 @@ class BridgeInterfaceDriver(LinuxInterfaceDriver):
|
|||
root_veth.link.set_up()
|
||||
ns_veth.link.set_up()
|
||||
|
||||
@device_name_synchronized
|
||||
def unplug(self, device_name, bridge=None, namespace=None, prefix=None):
|
||||
"""Unplug the interface."""
|
||||
device = ip_lib.IPDevice(device_name, namespace)
|
||||
|
|
|
@ -500,7 +500,7 @@ class ServiceInstanceManager(object):
|
|||
port = self._get_service_port()
|
||||
port = self._add_fixed_ips_to_service_port(port)
|
||||
interface_name = self.vif_driver.get_device_name(port)
|
||||
self.vif_driver.plug(port['id'], interface_name, port['mac_address'])
|
||||
self.vif_driver.plug(interface_name, port['id'], port['mac_address'])
|
||||
ip_cidrs = []
|
||||
for fixed_ip in port['fixed_ips']:
|
||||
subnet = self.neutron_api.get_subnet(fixed_ip['subnet_id'])
|
||||
|
|
|
@ -139,8 +139,8 @@ class TestOVSInterfaceDriver(TestBase):
|
|||
with mock.patch.object(utils, 'execute') as execute:
|
||||
ovs = interface.OVSInterfaceDriver()
|
||||
self.device_exists.side_effect = device_exists
|
||||
ovs.plug('port-1234',
|
||||
'tap0',
|
||||
ovs.plug('tap0',
|
||||
'port-1234',
|
||||
'aa:bb:cc:dd:ee:ff',
|
||||
bridge=bridge,
|
||||
namespace=namespace)
|
||||
|
@ -193,8 +193,8 @@ class TestBridgeInterfaceDriver(TestBase):
|
|||
self.device_exists.side_effect = device_exists
|
||||
br = interface.BridgeInterfaceDriver()
|
||||
mac_address = 'aa:bb:cc:dd:ee:ff'
|
||||
br.plug('port-1234',
|
||||
'ns-0',
|
||||
br.plug('ns-0',
|
||||
'port-1234',
|
||||
mac_address,
|
||||
namespace=namespace)
|
||||
|
||||
|
|
|
@ -668,7 +668,7 @@ class ServiceInstanceManagerTestCase(test.TestCase):
|
|||
self._manager.vif_driver.get_device_name.assert_called_once_with(
|
||||
fake_port)
|
||||
self._manager.vif_driver.plug.assert_called_once_with(
|
||||
fake_port['id'], interface_name, fake_port['mac_address'])
|
||||
interface_name, fake_port['id'], fake_port['mac_address'])
|
||||
self._manager.neutron_api.get_subnet.assert_called_once_with(
|
||||
fake_subnet['id'])
|
||||
self._manager.vif_driver.init_l3.assert_called_once_with(
|
||||
|
|
Loading…
Reference in New Issue