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
This commit is contained in:
Eric Harney 2014-01-23 11:57:17 -05:00
parent d6f7101f2d
commit e9722f09df
2 changed files with 22 additions and 12 deletions

View File

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

View File

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