From 86643780bc2cf1f7a467a7c65aab64e3110d031e Mon Sep 17 00:00:00 2001
From: Edward Hope-Morley <edward.hope-morley@canonical.com>
Date: Sun, 15 Jun 2014 21:30:19 +0100
Subject: [PATCH] Removes unecessary call to rbd.Image

Removed duplicate and unprotected call to rbd.Image
in rbd driver which can result in an uncaught exception
if the rbd image no longer exists.

Also added unit test for _delete_backup_snaps()

Closes-Bug: 1323203
Change-Id: I89eedb0d0276a985b4d496093df2eee2ce067ec1
---
 cinder/tests/test_rbd.py     | 11 +++++++++++
 cinder/volume/drivers/rbd.py | 20 ++++++++------------
 2 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/cinder/tests/test_rbd.py b/cinder/tests/test_rbd.py
index c2c572c3b..43b43c224 100644
--- a/cinder/tests/test_rbd.py
+++ b/cinder/tests/test_rbd.py
@@ -201,6 +201,17 @@ class RBDTestCase(test.TestCase):
             client.__exit__.assert_called_once()
             mock_supports_layering.assert_called_once()
 
+    @common_mocks
+    def test_delete_backup_snaps(self):
+        self.driver.rbd.Image.remove_snap = mock.Mock()
+        with mock.patch.object(self.driver, '_get_backup_snaps') as \
+                mock_get_backup_snaps:
+            mock_get_backup_snaps.return_value = [{'name': 'snap1'}]
+            rbd_image = self.driver.rbd.Image()
+            self.driver._delete_backup_snaps(rbd_image)
+            mock_get_backup_snaps.assert_called_once_with(rbd_image)
+            self.assertTrue(self.driver.rbd.Image.remove_snap.called)
+
     @common_mocks
     def test_delete_volume(self):
         client = self.mock_client.return_value
diff --git a/cinder/volume/drivers/rbd.py b/cinder/volume/drivers/rbd.py
index ee7ddce95..11bf60887 100644
--- a/cinder/volume/drivers/rbd.py
+++ b/cinder/volume/drivers/rbd.py
@@ -518,17 +518,13 @@ class RBDDriver(driver.VolumeDriver):
         if int(volume['size']):
             self._resize(volume)
 
-    def _delete_backup_snaps(self, client, volume_name):
-        rbd_image = self.rbd.Image(client.ioctx, volume_name)
-        try:
-            backup_snaps = self._get_backup_snaps(rbd_image)
-            if backup_snaps:
-                for snap in backup_snaps:
-                    rbd_image.remove_snap(snap['name'])
-            else:
-                LOG.debug(_("volume has no backup snaps"))
-        finally:
-            rbd_image.close()
+    def _delete_backup_snaps(self, rbd_image):
+        backup_snaps = self._get_backup_snaps(rbd_image)
+        if backup_snaps:
+            for snap in backup_snaps:
+                rbd_image.remove_snap(snap['name'])
+        else:
+            LOG.debug(_("volume has no backup snaps"))
 
     def _get_clone_info(self, volume, volume_name, snap=None):
         """If volume is a clone, return its parent info.
@@ -602,7 +598,7 @@ class RBDDriver(driver.VolumeDriver):
             parent = None
 
             # Ensure any backup snapshots are deleted
-            self._delete_backup_snaps(client, volume_name)
+            self._delete_backup_snaps(rbd_image)
 
             # If the volume has non-clone snapshots this delete is expected to
             # raise VolumeIsBusy so do so straight away.
-- 
2.45.2