From fe675be8329a29674436f6e9bfcd5eac87989249 Mon Sep 17 00:00:00 2001 From: Valeriy Ponomaryov Date: Fri, 22 Aug 2014 06:05:55 -0400 Subject: [PATCH] Updated usage of locks Changes: - replaced direct usage of sychronized func from lockutils with local wrapper, that provides prefix; - 'lock_path' is not provided anymore as hardcode, defined 'lock_path' in config is expected; - added MANILA_LOCK_PATH to devstack plugin, to be able to set path before installation. Change-Id: I6518df0a6d32f1021b19a48f928f9ec8b00140b2 Closes-Bug: #1347915 --- .gitignore | 3 +- contrib/devstack/lib/manila | 3 ++ manila/network/linux/interface.py | 4 +-- manila/share/drivers/generic.py | 9 +++--- manila/share/drivers/service_instance.py | 36 ++++++++++-------------- manila/share/manager.py | 7 +++-- manila/tests/test_service_instance.py | 7 ++--- manila/tests/test_share_generic.py | 5 ++-- 8 files changed, 33 insertions(+), 41 deletions(-) diff --git a/.gitignore b/.gitignore index a27388a890..a30cc7f0c4 100644 --- a/.gitignore +++ b/.gitignore @@ -38,5 +38,4 @@ doc/build .pydevproject # Lock dirs and files -service_instance_locks -attach_detach_locks +manila_locks diff --git a/contrib/devstack/lib/manila b/contrib/devstack/lib/manila index aa9a87e47f..fded8d43cc 100644 --- a/contrib/devstack/lib/manila +++ b/contrib/devstack/lib/manila @@ -34,6 +34,7 @@ MANILACLIENT_BRANCH=master # set up default directories MANILA_DIR=${MANILA_DIR:=$DEST/manila} +MANILA_LOCK_PATH=${MANILA_LOCK_PATH:=$MANILA_DIR/manila_locks} MANILACLIENT_DIR=${MANILACLIENT_DIR:=$DEST/python-manilaclient} MANILA_STATE_PATH=${MANILA_STATE_PATH:=$DATA_DIR/manila} MANILA_MNT_DIR=${MANILA_MNT_DIR:=$MANILA_STATE_PATH/mnt} @@ -179,6 +180,8 @@ function configure_manila { iniset $MANILA_CONF DEFAULT cinder_admin_password $SERVICE_PASSWORD iniset $MANILA_CONF DEFAULT neutron_admin_password $SERVICE_PASSWORD + iniset $MANILA_CONF DEFAULT lock_path $MANILA_LOCK_PATH + add_share_backend $MANILA_BACKEND1_CONFIG_GROUP_NAME $MANILA_SHARE_BACKEND1_NAME enabled_backends=$MANILA_BACKEND1_CONFIG_GROUP_NAME diff --git a/manila/network/linux/interface.py b/manila/network/linux/interface.py index 6a7165c158..fbda83ad3b 100644 --- a/manila/network/linux/interface.py +++ b/manila/network/linux/interface.py @@ -22,7 +22,6 @@ 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 @@ -45,8 +44,7 @@ def device_name_synchronized(f): def wrapped_func(self, *args, **kwargs): device_name = "device_name_%s" % args[0] - @lockutils.synchronized(device_name, external=True, - lock_path="service_instance_locks") + @utils.synchronized("linux_interface_%s" % device_name, external=True) def source_func(self, *args, **kwargs): return f(self, *args, **kwargs) diff --git a/manila/share/drivers/generic.py b/manila/share/drivers/generic.py index 1d837addc3..59fa50b36a 100644 --- a/manila/share/drivers/generic.py +++ b/manila/share/drivers/generic.py @@ -27,7 +27,6 @@ from manila import context from manila import exception from manila.openstack.common import excutils from manila.openstack.common import importutils -from manila.openstack.common import lockutils from manila.openstack.common import log as logging from manila.openstack.common import processutils from manila.share import driver @@ -272,8 +271,8 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver): def _attach_volume(self, context, share, instance_id, volume): """Attaches cinder volume to service vm.""" - @lockutils.synchronized(instance_id, external=True, - lock_path="attach_detach_locks") + @utils.synchronized( + "generic_driver_attach_detach_%s" % instance_id, external=True) def do_attach(volume): if volume['status'] == 'in-use': attached_volumes = [vol.id for vol in @@ -337,8 +336,8 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver): """Detaches cinder volume from service vm.""" instance_id = server_details['instance_id'] - @lockutils.synchronized(instance_id, external=True, - lock_path="attach_detach_locks") + @utils.synchronized( + "generic_driver_attach_detach_%s" % instance_id, external=True) def do_detach(): attached_volumes = [vol.id for vol in self.compute_api.instance_volumes_list( diff --git a/manila/share/drivers/service_instance.py b/manila/share/drivers/service_instance.py index 05a146b782..26b623df9e 100644 --- a/manila/share/drivers/service_instance.py +++ b/manila/share/drivers/service_instance.py @@ -31,7 +31,6 @@ from manila import exception from manila.network.linux import ip_lib from manila.network.neutron import api as neutron from manila.openstack.common import importutils -from manila.openstack.common import lockutils from manila.openstack.common import log as logging from manila import utils @@ -159,8 +158,7 @@ class ServiceInstanceManager(object): "path_to_private_key") self.path_to_public_key = self.get_config_option("path_to_public_key") - @lockutils.synchronized("_get_service_network", external=True, - lock_path="service_instance_locks") + @utils.synchronized("service_instance_get_service_network", external=True) def _get_service_network(self): """Finds existing or creates new service network.""" service_network_name = self.get_config_option("service_network_name") @@ -202,8 +200,8 @@ class ServiceInstanceManager(object): raise exception.ServiceInstanceException(msg) return net_ips[0] - @lockutils.synchronized("_get_or_create_security_group", - external=True, lock_path="service_instance_locks") + @utils.synchronized( + "service_instance_get_or_create_security_group", external=True) def _get_or_create_security_group(self, context, name=None, description=None): """Get or create security group for service_instance. @@ -301,8 +299,7 @@ class ServiceInstanceManager(object): 'service_instance_password'), 'username': self.get_config_option('service_instance_user')} - @lockutils.synchronized("_get_key", external=True, - lock_path="service_instance_locks") + @utils.synchronized("service_instance_get_key", external=True) def _get_key(self, context): """Get ssh key. @@ -441,8 +438,8 @@ class ServiceInstanceManager(object): time.sleep(5) return False - @lockutils.synchronized("_setup_network_for_instance", external=True, - lock_path="service_instance_locks") + @utils.synchronized( + "service_instance_setup_network_for_instance", external=True) def _setup_network_for_instance(self, neutron_net_id, neutron_subnet_id): """Sets up network for service vm.""" @@ -477,8 +474,7 @@ class ServiceInstanceManager(object): device_owner='manila') return service_subnet['id'], private_router['id'], port['id'] - @lockutils.synchronized("_get_private_router", external=True, - lock_path="service_instance_locks") + @utils.synchronized("service_instance_get_private_router", external=True) def _get_private_router(self, neutron_net_id, neutron_subnet_id): """Returns router attached to private subnet gateway.""" private_subnet = self.neutron_api.get_subnet(neutron_subnet_id) @@ -526,8 +522,8 @@ class ServiceInstanceManager(object): # here we are checking for garbage devices from removed service port self._remove_outdated_interfaces(device) - @lockutils.synchronized("_remove_outdated_interfaces", external=True, - lock_path="service_instance_locks") + @utils.synchronized( + "service_instance_remove_outdated_interfaces", external=True) def _remove_outdated_interfaces(self, device): """Finds and removes unused network device.""" list_dev = [] @@ -546,8 +542,7 @@ class ServiceInstanceManager(object): if device_cidr_set & cidr_set: self.vif_driver.unplug(dev_name) - @lockutils.synchronized("_get_service_port", external=True, - lock_path="service_instance_locks") + @utils.synchronized("service_instance_get_service_port", external=True) def _get_service_port(self): """Find or creates service neutron port. @@ -575,8 +570,8 @@ class ServiceInstanceManager(object): port = ports[0] return port - @lockutils.synchronized("_add_fixed_ips_to_service_port", external=True, - lock_path="service_instance_locks") + @utils.synchronized( + "service_instance_add_fixed_ips_to_service_port", external=True) def _add_fixed_ips_to_service_port(self, port): network = self.neutron_api.get_network(self.service_network_id) subnets = set(network['subnets']) @@ -630,15 +625,14 @@ class ServiceInstanceManager(object): 'router_id': router_id}) self.neutron_api.update_subnet(subnet_id, '') - @lockutils.synchronized("_get_all_service_subnets", external=True, - lock_path="service_instance_locks") + @utils.synchronized( + "service_instance_get_all_service_subnets", external=True) def _get_all_service_subnets(self): service_network = self.neutron_api.get_network(self.service_network_id) return [self.neutron_api.get_subnet(subnet_id) for subnet_id in service_network['subnets']] - @lockutils.synchronized("_get_service_subnet", external=True, - lock_path="service_instance_locks") + @utils.synchronized("service_instance_get_service_subnet", external=True) def _get_service_subnet(self, subnet_name): all_service_subnets = self._get_all_service_subnets() service_subnets = [subnet for subnet in all_service_subnets diff --git a/manila/share/manager.py b/manila/share/manager.py index 3b1bde0c98..96f28b6ddc 100644 --- a/manila/share/manager.py +++ b/manila/share/manager.py @@ -26,11 +26,11 @@ from manila import manager from manila import network from manila.openstack.common import excutils from manila.openstack.common import importutils -from manila.openstack.common import lockutils from manila.openstack.common import log as logging from manila.openstack.common import timeutils from manila import quota import manila.share.configuration +from manila import utils from oslo.config import cfg @@ -116,7 +116,7 @@ class ShareManager(manager.SchedulerDependentManager): share updated with share_server_id. """ - @lockutils.synchronized(share_network_id) + @utils.synchronized("share_manager_%s" % share_network_id) def _provide_share_server_for_share(): exist = False try: @@ -424,7 +424,8 @@ class ShareManager(manager.SchedulerDependentManager): def delete_share_server(self, context, share_server): - @lockutils.synchronized(share_server['share_network_id']) + @utils.synchronized( + "share_manager_%s" % share_server['share_network_id']) def _teardown_server(): # NOTE(vponomaryov): Verify that there are no dependent shares. # Without this verification we can get here exception in next case: diff --git a/manila/tests/test_service_instance.py b/manila/tests/test_service_instance.py index aff4beb92c..2ecd7e0af9 100644 --- a/manila/tests/test_service_instance.py +++ b/manila/tests/test_service_instance.py @@ -19,17 +19,16 @@ import copy import os import mock +from oslo.config import cfg from manila import context from manila import exception -from manila.openstack.common import lockutils from manila.share.drivers import service_instance from manila import test from manila.tests.db import fakes as db_fakes from manila.tests import fake_compute from manila.tests import fake_network - -from oslo.config import cfg +from manila import utils CONF = cfg.CONF @@ -69,7 +68,7 @@ class ServiceInstanceManagerTestCase(test.TestCase): self._manager.admin_context = self._context self._manager._execute = mock.Mock(return_value=('', '')) self._manager.vif_driver = mock.Mock() - self.stubs.Set(lockutils, 'synchronized', + self.stubs.Set(utils, 'synchronized', mock.Mock(return_value=lambda f: f)) self.stubs.Set(service_instance.os.path, 'exists', mock.Mock(return_value=True)) diff --git a/manila/tests/test_share_generic.py b/manila/tests/test_share_generic.py index 302309b4b9..d27810623b 100644 --- a/manila/tests/test_share_generic.py +++ b/manila/tests/test_share_generic.py @@ -23,7 +23,6 @@ from oslo.config import cfg from manila import compute from manila import context from manila import exception -from manila.openstack.common import lockutils import manila.share.configuration from manila.share.drivers import generic from manila import test @@ -32,9 +31,9 @@ from manila.tests import fake_compute from manila.tests import fake_service_instance from manila.tests import fake_utils from manila.tests import fake_volume +from manila import utils from manila import volume - CONF = cfg.CONF @@ -115,7 +114,7 @@ class GenericShareDriverTestCase(test.TestCase): share_network_id=self.fake_sn["id"], old_server_ip="fake") self._driver._ssh_exec = mock.Mock(return_value=('', '')) - self.stubs.Set(lockutils, 'synchronized', + self.stubs.Set(utils, 'synchronized', mock.Mock(return_value=lambda f: f)) self.stubs.Set(generic.os.path, 'exists', mock.Mock(return_value=True)) self._driver._helpers = {