openstack
/
manila
Code Issues Proposed changes

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
This commit is contained in:
Valeriy Ponomaryov 2014-08-22 06:05:55 -04:00
parent 369bd6d497
commit fe675be832
8 changed files with 33 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
.gitignore vendored
View File

@ -38,5 +38,4 @@ doc/build
.pydevproject .pydevproject
# Lock dirs and files # Lock dirs and files
service_instance_locks manila_locks
attach_detach_locks

3
contrib/devstack/lib/manila
View File

@ -34,6 +34,7 @@ MANILACLIENT_BRANCH=master
# set up default directories # set up default directories
MANILA_DIR=${MANILA_DIR:=$DEST/manila} MANILA_DIR=${MANILA_DIR:=$DEST/manila}
MANILA_LOCK_PATH=${MANILA_LOCK_PATH:=$MANILA_DIR/manila_locks}
MANILACLIENT_DIR=${MANILACLIENT_DIR:=$DEST/python-manilaclient} MANILACLIENT_DIR=${MANILACLIENT_DIR:=$DEST/python-manilaclient}
MANILA_STATE_PATH=${MANILA_STATE_PATH:=$DATA_DIR/manila} MANILA_STATE_PATH=${MANILA_STATE_PATH:=$DATA_DIR/manila}
MANILA_MNT_DIR=${MANILA_MNT_DIR:=$MANILA_STATE_PATH/mnt} 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 cinder_admin_password $SERVICE_PASSWORD
iniset $MANILA_CONF DEFAULT neutron_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 add_share_backend $MANILA_BACKEND1_CONFIG_GROUP_NAME $MANILA_SHARE_BACKEND1_NAME
enabled_backends=$MANILA_BACKEND1_CONFIG_GROUP_NAME enabled_backends=$MANILA_BACKEND1_CONFIG_GROUP_NAME

4
manila/network/linux/interface.py
View File

@ -22,7 +22,6 @@ import six
from manila import exception from manila import exception
from manila.network.linux import ip_lib from manila.network.linux import ip_lib
from manila.network.linux import ovs_lib from manila.network.linux import ovs_lib
from manila.openstack.common import lockutils
from manila.openstack.common import log as logging from manila.openstack.common import log as logging
from manila import utils from manila import utils
@ -45,8 +44,7 @@ def device_name_synchronized(f):
def wrapped_func(self, *args, **kwargs): def wrapped_func(self, *args, **kwargs):
device_name = "device_name_%s" % args[0] device_name = "device_name_%s" % args[0]
@lockutils.synchronized(device_name, external=True, @utils.synchronized("linux_interface_%s" % device_name, external=True)
lock_path="service_instance_locks")
def source_func(self, *args, **kwargs): def source_func(self, *args, **kwargs):
return f(self, *args, **kwargs) return f(self, *args, **kwargs)

9
manila/share/drivers/generic.py
View File

@ -27,7 +27,6 @@ from manila import context
from manila import exception from manila import exception
from manila.openstack.common import excutils from manila.openstack.common import excutils
from manila.openstack.common import importutils 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 log as logging
from manila.openstack.common import processutils from manila.openstack.common import processutils
from manila.share import driver from manila.share import driver
@ -272,8 +271,8 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver):
def _attach_volume(self, context, share, instance_id, volume): def _attach_volume(self, context, share, instance_id, volume):
"""Attaches cinder volume to service vm.""" """Attaches cinder volume to service vm."""
@lockutils.synchronized(instance_id, external=True, @utils.synchronized(
lock_path="attach_detach_locks") "generic_driver_attach_detach_%s" % instance_id, external=True)
def do_attach(volume): def do_attach(volume):
if volume['status'] == 'in-use': if volume['status'] == 'in-use':
attached_volumes = [vol.id for vol in attached_volumes = [vol.id for vol in
@ -337,8 +336,8 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver):
"""Detaches cinder volume from service vm.""" """Detaches cinder volume from service vm."""
instance_id = server_details['instance_id'] instance_id = server_details['instance_id']
@lockutils.synchronized(instance_id, external=True, @utils.synchronized(
lock_path="attach_detach_locks") "generic_driver_attach_detach_%s" % instance_id, external=True)
def do_detach(): def do_detach():
attached_volumes = [vol.id for vol in attached_volumes = [vol.id for vol in
self.compute_api.instance_volumes_list( self.compute_api.instance_volumes_list(

36
manila/share/drivers/service_instance.py
View File

@ -31,7 +31,6 @@ from manila import exception
from manila.network.linux import ip_lib from manila.network.linux import ip_lib
from manila.network.neutron import api as neutron from manila.network.neutron import api as neutron
from manila.openstack.common import importutils 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 log as logging
from manila import utils from manila import utils
@ -159,8 +158,7 @@ class ServiceInstanceManager(object):
"path_to_private_key") "path_to_private_key")
self.path_to_public_key = self.get_config_option("path_to_public_key") self.path_to_public_key = self.get_config_option("path_to_public_key")
@lockutils.synchronized("_get_service_network", external=True, @utils.synchronized("service_instance_get_service_network", external=True)
lock_path="service_instance_locks")
def _get_service_network(self): def _get_service_network(self):
"""Finds existing or creates new service network.""" """Finds existing or creates new service network."""
service_network_name = self.get_config_option("service_network_name") service_network_name = self.get_config_option("service_network_name")
@ -202,8 +200,8 @@ class ServiceInstanceManager(object):
raise exception.ServiceInstanceException(msg) raise exception.ServiceInstanceException(msg)
return net_ips[0] return net_ips[0]
@lockutils.synchronized("_get_or_create_security_group", @utils.synchronized(
external=True, lock_path="service_instance_locks") "service_instance_get_or_create_security_group", external=True)
def _get_or_create_security_group(self, context, name=None, def _get_or_create_security_group(self, context, name=None,
description=None): description=None):
"""Get or create security group for service_instance. """Get or create security group for service_instance.
@ -301,8 +299,7 @@ class ServiceInstanceManager(object):
'service_instance_password'), 'service_instance_password'),
'username': self.get_config_option('service_instance_user')} 'username': self.get_config_option('service_instance_user')}
@lockutils.synchronized("_get_key", external=True, @utils.synchronized("service_instance_get_key", external=True)
lock_path="service_instance_locks")
def _get_key(self, context): def _get_key(self, context):
"""Get ssh key. """Get ssh key.
@ -441,8 +438,8 @@ class ServiceInstanceManager(object):
time.sleep(5) time.sleep(5)
return False return False
@lockutils.synchronized("_setup_network_for_instance", external=True, @utils.synchronized(
lock_path="service_instance_locks") "service_instance_setup_network_for_instance", external=True)
def _setup_network_for_instance(self, neutron_net_id, neutron_subnet_id): def _setup_network_for_instance(self, neutron_net_id, neutron_subnet_id):
"""Sets up network for service vm.""" """Sets up network for service vm."""
@ -477,8 +474,7 @@ class ServiceInstanceManager(object):
device_owner='manila') device_owner='manila')
return service_subnet['id'], private_router['id'], port['id'] return service_subnet['id'], private_router['id'], port['id']
@lockutils.synchronized("_get_private_router", external=True, @utils.synchronized("service_instance_get_private_router", external=True)
lock_path="service_instance_locks")
def _get_private_router(self, neutron_net_id, neutron_subnet_id): def _get_private_router(self, neutron_net_id, neutron_subnet_id):
"""Returns router attached to private subnet gateway.""" """Returns router attached to private subnet gateway."""
private_subnet = self.neutron_api.get_subnet(neutron_subnet_id) 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 # here we are checking for garbage devices from removed service port
self._remove_outdated_interfaces(device) self._remove_outdated_interfaces(device)
@lockutils.synchronized("_remove_outdated_interfaces", external=True, @utils.synchronized(
lock_path="service_instance_locks") "service_instance_remove_outdated_interfaces", external=True)
def _remove_outdated_interfaces(self, device): def _remove_outdated_interfaces(self, device):
"""Finds and removes unused network device.""" """Finds and removes unused network device."""
list_dev = [] list_dev = []
@ -546,8 +542,7 @@ class ServiceInstanceManager(object):
if device_cidr_set & cidr_set: if device_cidr_set & cidr_set:
self.vif_driver.unplug(dev_name) self.vif_driver.unplug(dev_name)
@lockutils.synchronized("_get_service_port", external=True, @utils.synchronized("service_instance_get_service_port", external=True)
lock_path="service_instance_locks")
def _get_service_port(self): def _get_service_port(self):
"""Find or creates service neutron port. """Find or creates service neutron port.
@ -575,8 +570,8 @@ class ServiceInstanceManager(object):
port = ports[0] port = ports[0]
return port return port
@lockutils.synchronized("_add_fixed_ips_to_service_port", external=True, @utils.synchronized(
lock_path="service_instance_locks") "service_instance_add_fixed_ips_to_service_port", external=True)
def _add_fixed_ips_to_service_port(self, port): def _add_fixed_ips_to_service_port(self, port):
network = self.neutron_api.get_network(self.service_network_id) network = self.neutron_api.get_network(self.service_network_id)
subnets = set(network['subnets']) subnets = set(network['subnets'])
@ -630,15 +625,14 @@ class ServiceInstanceManager(object):
'router_id': router_id}) 'router_id': router_id})
self.neutron_api.update_subnet(subnet_id, '') self.neutron_api.update_subnet(subnet_id, '')
@lockutils.synchronized("_get_all_service_subnets", external=True, @utils.synchronized(
lock_path="service_instance_locks") "service_instance_get_all_service_subnets", external=True)
def _get_all_service_subnets(self): def _get_all_service_subnets(self):
service_network = self.neutron_api.get_network(self.service_network_id) service_network = self.neutron_api.get_network(self.service_network_id)
return [self.neutron_api.get_subnet(subnet_id) return [self.neutron_api.get_subnet(subnet_id)
for subnet_id in service_network['subnets']] for subnet_id in service_network['subnets']]
@lockutils.synchronized("_get_service_subnet", external=True, @utils.synchronized("service_instance_get_service_subnet", external=True)
lock_path="service_instance_locks")
def _get_service_subnet(self, subnet_name): def _get_service_subnet(self, subnet_name):
all_service_subnets = self._get_all_service_subnets() all_service_subnets = self._get_all_service_subnets()
service_subnets = [subnet for subnet in all_service_subnets service_subnets = [subnet for subnet in all_service_subnets

7
manila/share/manager.py
View File

@ -26,11 +26,11 @@ from manila import manager
from manila import network from manila import network
from manila.openstack.common import excutils from manila.openstack.common import excutils
from manila.openstack.common import importutils 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 log as logging
from manila.openstack.common import timeutils from manila.openstack.common import timeutils
from manila import quota from manila import quota
import manila.share.configuration import manila.share.configuration
from manila import utils
from oslo.config import cfg from oslo.config import cfg
@ -116,7 +116,7 @@ class ShareManager(manager.SchedulerDependentManager):
share updated with share_server_id. 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(): def _provide_share_server_for_share():
exist = False exist = False
try: try:
@ -424,7 +424,8 @@ class ShareManager(manager.SchedulerDependentManager):
def delete_share_server(self, context, share_server): 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(): def _teardown_server():
# NOTE(vponomaryov): Verify that there are no dependent shares. # NOTE(vponomaryov): Verify that there are no dependent shares.
# Without this verification we can get here exception in next case: # Without this verification we can get here exception in next case:

7
manila/tests/test_service_instance.py
View File

@ -19,17 +19,16 @@ import copy
import os import os
import mock import mock
from oslo.config import cfg
from manila import context from manila import context
from manila import exception from manila import exception
from manila.openstack.common import lockutils
from manila.share.drivers import service_instance from manila.share.drivers import service_instance
from manila import test from manila import test
from manila.tests.db import fakes as db_fakes from manila.tests.db import fakes as db_fakes
from manila.tests import fake_compute from manila.tests import fake_compute
from manila.tests import fake_network from manila.tests import fake_network
from manila import utils
from oslo.config import cfg
CONF = cfg.CONF CONF = cfg.CONF
@ -69,7 +68,7 @@ class ServiceInstanceManagerTestCase(test.TestCase):
self._manager.admin_context = self._context self._manager.admin_context = self._context
self._manager._execute = mock.Mock(return_value=('', '')) self._manager._execute = mock.Mock(return_value=('', ''))
self._manager.vif_driver = mock.Mock() self._manager.vif_driver = mock.Mock()
self.stubs.Set(lockutils, 'synchronized', self.stubs.Set(utils, 'synchronized',
mock.Mock(return_value=lambda f: f)) mock.Mock(return_value=lambda f: f))
self.stubs.Set(service_instance.os.path, 'exists', self.stubs.Set(service_instance.os.path, 'exists',
mock.Mock(return_value=True)) mock.Mock(return_value=True))

5
manila/tests/test_share_generic.py
View File

@ -23,7 +23,6 @@ from oslo.config import cfg
from manila import compute from manila import compute
from manila import context from manila import context
from manila import exception from manila import exception
from manila.openstack.common import lockutils
import manila.share.configuration import manila.share.configuration
from manila.share.drivers import generic from manila.share.drivers import generic
from manila import test 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_service_instance
from manila.tests import fake_utils from manila.tests import fake_utils
from manila.tests import fake_volume from manila.tests import fake_volume
from manila import utils
from manila import volume from manila import volume
CONF = cfg.CONF CONF = cfg.CONF
@ -115,7 +114,7 @@ class GenericShareDriverTestCase(test.TestCase):
share_network_id=self.fake_sn["id"], old_server_ip="fake") share_network_id=self.fake_sn["id"], old_server_ip="fake")
self._driver._ssh_exec = mock.Mock(return_value=('', '')) 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)) mock.Mock(return_value=lambda f: f))
self.stubs.Set(generic.os.path, 'exists', mock.Mock(return_value=True)) self.stubs.Set(generic.os.path, 'exists', mock.Mock(return_value=True))
self._driver._helpers = { self._driver._helpers = {