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:
parent
d6f7101f2d
commit
e9722f09df
|
@ -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()
|
||||
|
||||
|
|
|
@ -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
|
||||
|
|
Loading…
Reference in New Issue