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

(cherry picked from commit e9722f09df)

Note: Effectively includes most changes from "8772714 Pylint fixes ..."
to resolve conflicts.
(cherry picked from commit 8772714b4fcb2d05ae7f0bfe5cfd0fa660ab9100)

Conflicts:
        cinder/volume/drivers/glusterfs.py

Change-Id: I84ca34b201c10644faa047f1c9274c14bcdd0359

cinder/tests/test_glusterfs.py

@@ -641,8 +641,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')
@@ -675,7 +675,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'}
@@ -684,7 +684,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()
 
@@ -1632,9 +1632,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'],
@@ -1650,11 +1650,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()
 

cinder/volume/drivers/glusterfs.py

@@ -186,14 +186,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']}
 
@@ -211,6 +211,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.
 
@@ -287,6 +288,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,
@@ -455,8 +461,7 @@ class GlusterfsDriver(nfs.RemoteFsDriver):
         LOG.debug(_('volume id: %s') % snapshot['volume_id'])
 
         path_to_disk = self._local_path_volume(snapshot['volume'])
-        snap_id = snapshot['id']
-        self._create_snapshot(snapshot, path_to_disk, snap_id)
+        self._create_snapshot_offline(snapshot, path_to_disk)
 
     def _create_qcow2_snap_file(self, snapshot, backing_filename,
                                 new_snap_path):
@@ -485,7 +490,7 @@ class GlusterfsDriver(nfs.RemoteFsDriver):
                    new_snap_path]
         self._execute(*command, run_as_root=True)
 
-    def _create_snapshot(self, snapshot, path_to_disk, snap_id):
+    def _create_snapshot_offline(self, snapshot, path_to_disk):
         """Create snapshot (offline case)."""
 
         # Requires volume status = 'available'
@@ -540,6 +545,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