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
# Lock dirs and files
service_instance_locks
attach_detach_locks
manila_locks

3
contrib/devstack/lib/manila
View File

@ -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

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

@ -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)

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

@ -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(

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.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

7
manila/share/manager.py
View File

@ -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:

7
manila/tests/test_service_instance.py
View File

@ -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))

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 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 = {