GlusterFS: Fix deadlock in volume clone

The create_cloned_volume path could deadlock due to
create_cloned_volume and create/delete_snapshot using the same
lock for synchronization.

Refactor the calls to create/delete snapshot to call the inner
method which does not use a lock.

Introduced by "06999f6 GlusterFS: Synchronize additional op..."

Related-Bug: 1267983
Closes-Bug: 1272092

Change-Id: I84ca34b201c10644faa047f1c9274c14bcdd0359

diff --git a/cinder/tests/test_glusterfs.py b/cinder/tests/test_glusterfs.py
index 9fa60efcb707c8474d66173669660b99384a582f..ed81dbb828a3d47e3550f10b132db1a63ccc1964 100644 (file)
--- a/cinder/tests/test_glusterfs.py
+++ b/cinder/tests/test_glusterfs.py
@@ -596,8 +596,8 @@ class GlusterFsDriverTestCase(test.TestCase):
     def test_create_cloned_volume(self):
         (mox, drv) = self._mox, self._driver
 
-        mox.StubOutWithMock(drv, 'create_snapshot')
-        mox.StubOutWithMock(drv, 'delete_snapshot')
+        mox.StubOutWithMock(drv, '_create_snapshot')
+        mox.StubOutWithMock(drv, '_delete_snapshot')
         mox.StubOutWithMock(drv, '_read_info_file')
         mox.StubOutWithMock(image_utils, 'convert_image')
         mox.StubOutWithMock(drv, '_copy_volume_from_snapshot')
@@ -630,7 +630,7 @@ class GlusterFsDriverTestCase(test.TestCase):
                     'id': 'tmp-snap-%s' % src_vref['id'],
                     'volume': src_vref}
 
-        drv.create_snapshot(snap_ref)
+        drv._create_snapshot(snap_ref)
 
         snap_info = {'active': volume_file,
                      snap_ref['id']: volume_path + '-clone'}
@@ -639,7 +639,7 @@ class GlusterFsDriverTestCase(test.TestCase):
 
         drv._copy_volume_from_snapshot(snap_ref, volume_ref, volume['size'])
 
-        drv.delete_snapshot(mox_lib.IgnoreArg())
+        drv._delete_snapshot(mox_lib.IgnoreArg())
 
         mox.ReplayAll()
 
@@ -1590,9 +1590,9 @@ class GlusterFsDriverTestCase(test.TestCase):
         volume = self._simple_volume('c1073000-0000-0000-0000-0000000c1073')
         src_volume = self._simple_volume()
 
-        mox.StubOutWithMock(drv, 'create_snapshot')
+        mox.StubOutWithMock(drv, '_create_snapshot')
         mox.StubOutWithMock(drv, '_copy_volume_from_snapshot')
-        mox.StubOutWithMock(drv, 'delete_snapshot')
+        mox.StubOutWithMock(drv, '_delete_snapshot')
 
         snap_ref = {'volume_name': src_volume['name'],
                     'name': 'clone-snap-%s' % src_volume['id'],
@@ -1608,11 +1608,11 @@ class GlusterFsDriverTestCase(test.TestCase):
                       'provider_location': volume['provider_location'],
                       'name': 'volume-' + volume['id']}
 
-        drv.create_snapshot(snap_ref)
+        drv._create_snapshot(snap_ref)
         drv._copy_volume_from_snapshot(snap_ref,
                                        volume_ref,
                                        src_volume['size'])
-        drv.delete_snapshot(snap_ref)
+        drv._delete_snapshot(snap_ref)
 
         mox.ReplayAll()
 

diff --git a/cinder/volume/drivers/glusterfs.py b/cinder/volume/drivers/glusterfs.py
index 71ac852a04700b5b36e230b0276323c610d9f50d..c738be5a7b0d593ba95db918504abbd1e1bf915b 100644 (file)
--- a/cinder/volume/drivers/glusterfs.py
+++ b/cinder/volume/drivers/glusterfs.py
@@ -180,14 +180,14 @@ class GlusterfsDriver(nfs.RemoteFsDriver):
                          'volume_id': src_vref['id'],
                          'id': 'tmp-snap-%s' % src_vref['id'],
                          'volume': src_vref}
-        self.create_snapshot(temp_snapshot)
+        self._create_snapshot(temp_snapshot)
         try:
             self._copy_volume_from_snapshot(temp_snapshot,
                                             volume_info,
                                             src_vref['size'])
 
         finally:
-            self.delete_snapshot(temp_snapshot)
+            self._delete_snapshot(temp_snapshot)
 
         return {'provider_location': src_vref['provider_location']}
 
@@ -205,6 +205,7 @@ class GlusterfsDriver(nfs.RemoteFsDriver):
 
         return {'provider_location': volume['provider_location']}
 
+    @utils.synchronized('glusterfs', external=False)
     def create_volume_from_snapshot(self, volume, snapshot):
         """Creates a volume from a snapshot.
 
@@ -283,6 +284,11 @@ class GlusterfsDriver(nfs.RemoteFsDriver):
 
     @utils.synchronized('glusterfs', external=False)
     def create_snapshot(self, snapshot):
+        """Apply locking to the create snapshot operation."""
+
+        return self._create_snapshot(snapshot)
+
+    def _create_snapshot(self, snapshot):
         """Create a snapshot.
 
         If volume is attached, call to Nova to create snapshot,
@@ -451,7 +457,7 @@ class GlusterfsDriver(nfs.RemoteFsDriver):
         LOG.debug(_('volume id: %s') % snapshot['volume_id'])
 
         path_to_disk = self._local_path_volume(snapshot['volume'])
-        self._create_snapshot(snapshot, path_to_disk)
+        self._create_snapshot_offline(snapshot, path_to_disk)
 
     def _create_qcow2_snap_file(self, snapshot, backing_filename,
                                 new_snap_path):
@@ -480,7 +486,7 @@ class GlusterfsDriver(nfs.RemoteFsDriver):
                    new_snap_path]
         self._execute(*command, run_as_root=True)
 
-    def _create_snapshot(self, snapshot, path_to_disk):
+    def _create_snapshot_offline(self, snapshot, path_to_disk):
         """Create snapshot (offline case)."""
 
         # Requires volume status = 'available'
@@ -535,6 +541,10 @@ class GlusterfsDriver(nfs.RemoteFsDriver):
 
     @utils.synchronized('glusterfs', external=False)
     def delete_snapshot(self, snapshot):
+        """Apply locking to the delete snapshot operation."""
+        self._delete_snapshot(snapshot)
+
+    def _delete_snapshot(self, snapshot):
         """Delete a snapshot.
 
         If volume status is 'available', delete snapshot here in Cinder