From 63ed20b7e9d9a3856c7f2444473504ae6e634e4c Mon Sep 17 00:00:00 2001 From: "zhu.boxiang" Date: Fri, 28 Sep 2018 19:07:12 +0800 Subject: [PATCH] RBD: fix volume reference handling in clone logic This patch fixes a bug in volume cloning where the source volume is prematurely closed. If the destination volume requires flattening and an exception occurs during flattening, the code attempts to perform cleanup operations on an already closed volume. This resulted in a segmentation fault which causes cinder to restart. Co-authored-by: Jon Bernard Change-Id: Ib713aa91b775d8ec07ffdb24dfe1db1b6ecf2921 Closes-Bug: #1794956 (cherry picked from commit 394fbd7e20dbd8fb8cc58510657753432c43fa9b) (cherry picked from commit 7bfd3c4dcf528ba900d6179ff06c227ea1016176) (cherry picked from commit 47caae4cbbcacf1d81554af4ea2bfec01a0f7c54) (cherry picked from commit 5d62c72d3545ed59b4cb3300b5c2b75d0a443a99) --- cinder/tests/unit/volume/drivers/test_rbd.py | 11 +++++++---- cinder/volume/drivers/rbd.py | 3 +-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/cinder/tests/unit/volume/drivers/test_rbd.py b/cinder/tests/unit/volume/drivers/test_rbd.py index 3b50840e8f5..58f35b594fa 100644 --- a/cinder/tests/unit/volume/drivers/test_rbd.py +++ b/cinder/tests/unit/volume/drivers/test_rbd.py @@ -923,10 +923,13 @@ class RBDTestCase(test.TestCase): (self.mock_rbd.Image.return_value.protect_snap .assert_called_once_with('.'.join( (self.volume_b.name, 'clone_snap')))) + # We expect clone() to be called exactly once. self.assertEqual( 1, self.mock_rbd.RBD.return_value.clone.call_count) + # Without flattening, only the source volume is opened, + # so only one call to close() should occur. self.assertEqual( - 2, self.mock_rbd.Image.return_value.close.call_count) + 1, self.mock_rbd.Image.return_value.close.call_count) self.assertTrue(mock_get_clone_depth.called) mock_resize.assert_not_called() mock_enable_repl.assert_not_called() @@ -962,7 +965,7 @@ class RBDTestCase(test.TestCase): image.protect_snap.assert_called_once_with(name + '.clone_snap') self.assertEqual(1, self.mock_rbd.RBD.return_value.clone.call_count) self.assertEqual( - 2, self.mock_rbd.Image.return_value.close.call_count) + 1, self.mock_rbd.Image.return_value.close.call_count) mock_get_clone_depth.assert_called_once_with( self.mock_client().__enter__(), self.volume_a.name) mock_resize.assert_not_called() @@ -992,7 +995,7 @@ class RBDTestCase(test.TestCase): self.assertEqual( 1, self.mock_rbd.RBD.return_value.clone.call_count) self.assertEqual( - 2, self.mock_rbd.Image.return_value.close.call_count) + 1, self.mock_rbd.Image.return_value.close.call_count) self.assertTrue(mock_get_clone_depth.called) self.assertEqual( 1, mock_resize.call_count) @@ -1049,7 +1052,7 @@ class RBDTestCase(test.TestCase): # We expect the driver to close both volumes, so 2 is expected self.assertEqual( - 3, self.mock_rbd.Image.return_value.close.call_count) + 2, self.mock_rbd.Image.return_value.close.call_count) self.assertTrue(mock_get_clone_depth.called) mock_enable_repl.assert_not_called() diff --git a/cinder/volume/drivers/rbd.py b/cinder/volume/drivers/rbd.py index dcdb8ee6dc4..cf063c50aeb 100644 --- a/cinder/volume/drivers/rbd.py +++ b/cinder/volume/drivers/rbd.py @@ -595,6 +595,7 @@ class RBDDriver(driver.CloneableImageVD, except Exception as e: src_volume.unprotect_snap(clone_snap) src_volume.remove_snap(clone_snap) + src_volume.close() msg = (_("Failed to clone '%(src_vol)s@%(src_snap)s' to " "'%(dest)s', error: %(error)s") % {'src_vol': src_name, @@ -603,8 +604,6 @@ class RBDDriver(driver.CloneableImageVD, 'error': e}) LOG.exception(msg) raise exception.VolumeBackendAPIException(data=msg) - finally: - src_volume.close() depth = self._get_clone_depth(client, src_name) # If dest volume is a clone and rbd_max_clone_depth reached,