Merge "Use os-brick locking for volume attach and detach"

lower-constraints.txt

@@ -64,7 +64,7 @@ netifaces==0.10.4
 networkx==1.11
 numpy==1.14.2
 openstacksdk==0.35.0
-os-brick==2.6.1
+os-brick==2.6.2
 os-client-config==1.29.0
 os-resource-classes==0.4.0
 os-service-types==1.7.0

nova/tests/unit/virt/test_block_device.py

@@ -499,17 +499,16 @@ class TestDriverBlockDevice(test.NoDBTestCase):
             mock.patch.object(driver_bdm, '_get_volume', return_value=volume),
             mock.patch.object(self.virt_driver, 'get_volume_connector',
                               return_value=connector),
-            mock.patch('nova.utils.synchronized',
+            mock.patch('os_brick.initiator.utils.guard_connection'),
-                side_effect=lambda a: lambda f: lambda *args: f(*args)),
             mock.patch.object(self.volume_api, 'attachment_delete'),
-        ) as (mock_get_volume, mock_get_connector, mock_sync, vapi_attach_del):
+        ) as (mock_get_volume, mock_get_connector, mock_guard,
+              vapi_attach_del):
 
             driver_bdm.detach(elevated_context, instance,
                               self.volume_api, self.virt_driver,
                               attachment_id=attachment_id)
 
-            if include_shared_targets:
+            mock_guard.assert_called_once_with(volume)
-                mock_sync.assert_called_once_with((uuids.service_uuid))
             vapi_attach_del.assert_called_once_with(elevated_context,
                                                     attachment_id)
 

nova/virt/block_device.py

@@ -16,6 +16,7 @@ import functools
 import itertools
 
 from os_brick import encryptors
+from os_brick.initiator import utils as brick_utils
 from oslo_log import log as logging
 from oslo_serialization import jsonutils
 from oslo_utils import excutils
@@ -24,7 +25,6 @@ from oslo_utils import excutils
 from nova import block_device
 import nova.conf
 from nova import exception
-from nova import utils
 
 CONF = nova.conf.CONF
 
@@ -444,19 +444,10 @@ class DriverVolumeBlockDevice(DriverBlockDevice):
     def detach(self, context, instance, volume_api, virt_driver,
                attachment_id=None, destroy_bdm=False):
         volume = self._get_volume(context, volume_api, self.volume_id)
-        # Check to see if we need to lock based on the shared_targets value.
+        # Let OS-Brick handle high level locking that covers the local os-brick
-        # Default to False if the volume does not expose that value to maintain
+        # detach and the Cinder call to call unmap the volume.  Not all volume
-        # legacy behavior.
+        # backends or hosts require locking.
-        if volume.get('shared_targets', False):
+        with brick_utils.guard_connection(volume):
-            # Lock the detach call using the provided service_uuid.
-            @utils.synchronized(volume['service_uuid'])
-            def _do_locked_detach(*args, **_kwargs):
-                self._do_detach(*args, **_kwargs)
-
-            _do_locked_detach(context, instance, volume_api, virt_driver,
-                              attachment_id, destroy_bdm)
-        else:
-            # We don't need to (or don't know if we need to) lock.
             self._do_detach(context, instance, volume_api, virt_driver,
                             attachment_id, destroy_bdm)
 
@@ -649,19 +640,10 @@ class DriverVolumeBlockDevice(DriverBlockDevice):
         volume = self._get_volume(context, volume_api, self.volume_id)
         volume_api.check_availability_zone(context, volume,
                                            instance=instance)
-        # Check to see if we need to lock based on the shared_targets value.
+        # Let OS-Brick handle high level locking that covers the call to
-        # Default to False if the volume does not expose that value to maintain
+        # Cinder that exports & maps the volume, and for the local os-brick
-        # legacy behavior.
+        # attach.  Not all volume backends or hosts require locking.
-        if volume.get('shared_targets', False):
+        with brick_utils.guard_connection(volume):
-            # Lock the attach call using the provided service_uuid.
-            @utils.synchronized(volume['service_uuid'])
-            def _do_locked_attach(*args, **_kwargs):
-                self._do_attach(*args, **_kwargs)
-
-            _do_locked_attach(context, instance, volume, volume_api,
-                              virt_driver, do_driver_attach)
-        else:
-            # We don't need to (or don't know if we need to) lock.
             self._do_attach(context, instance, volume, volume_api,
                             virt_driver, do_driver_attach)
 

requirements.txt

@@ -54,7 +54,7 @@ rfc3986>=1.1.0 # Apache-2.0
 oslo.middleware>=3.31.0 # Apache-2.0
 psutil>=3.2.2 # BSD
 oslo.versionedobjects>=1.35.0 # Apache-2.0
-os-brick>=2.6.1 # Apache-2.0
+os-brick>=2.6.2 # Apache-2.0
 os-resource-classes>=0.4.0 # Apache-2.0
 os-traits>=0.16.0 # Apache-2.0
 os-vif>=1.14.0 # Apache-2.0