From 878007122f6c2590893daea35ab92bb51d624b03 Mon Sep 17 00:00:00 2001
From: Lin Yang <lin.a.yang@intel.com>
Date: Mon, 3 Aug 2015 15:28:15 +0800
Subject: [PATCH] Fix status comparison for attached volume backup

1) When do backup for an attached volume, its previous_status should be
'in-use' instead of 'in_use'.
2) Added more assertions in corresponding unittest case for this function
to make sure it enter this if-block. Previous test case was passed even
with the incorrect status.

Change-Id: I80385aa593aea5d749409bf591e5a3c5f8ed478f
Closes-Bug: #1480734
---
 cinder/tests/unit/test_volume.py | 29 +++++++++++++++++++++--------
 cinder/volume/driver.py          |  2 +-
 2 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/cinder/tests/unit/test_volume.py b/cinder/tests/unit/test_volume.py
index a01029073..c20930ade 100644
--- a/cinder/tests/unit/test_volume.py
+++ b/cinder/tests/unit/test_volume.py
@@ -5607,7 +5607,8 @@ class GenericVolumeDriverTestCase(DriverTestCase):
                                  mock_file_open,
                                  mock_temporary_chown):
         vol = tests_utils.create_volume(self.context)
-        vol['status'] = 'in-use'
+        vol['previous_status'] = 'in-use'
+        temp_vol = tests_utils.create_volume(self.context)
         self.context.user_id = 'fake'
         self.context.project_id = 'fake'
         backup = tests_utils.create_backup(self.context,
@@ -5620,11 +5621,11 @@ class GenericVolumeDriverTestCase(DriverTestCase):
         self.volume.driver._attach_volume = mock.MagicMock()
         self.volume.driver._detach_volume = mock.MagicMock()
         self.volume.driver.terminate_connection = mock.MagicMock()
-        self.volume.driver.create_snapshot = mock.MagicMock()
-        self.volume.driver.delete_snapshot = mock.MagicMock()
-        self.volume.driver.create_volume_from_snapshot = mock.MagicMock()
+        self.volume.driver._create_temp_cloned_volume = mock.MagicMock()
+        self.volume.driver._delete_volume = mock.MagicMock()
 
         mock_volume_get.return_value = vol
+        self.volume.driver._create_temp_cloned_volume.return_value = temp_vol
         mock_get_connector_properties.return_value = properties
         f = mock_file_open.return_value = file('/dev/null')
 
@@ -5635,6 +5636,10 @@ class GenericVolumeDriverTestCase(DriverTestCase):
                                          backup_service)
 
         mock_volume_get.assert_called_with(self.context, vol['id'])
+        self.volume.driver._create_temp_cloned_volume.assert_called_once_with(
+            self.context, vol)
+        self.volume.driver._delete_volume.assert_called_once_with(self.context,
+                                                                  temp_vol)
 
     def test_restore_backup(self):
         vol = tests_utils.create_volume(self.context)
@@ -6141,10 +6146,14 @@ class LVMVolumeDriverTestCase(DriverTestCase):
                                  mock_get_connector_properties,
                                  mock_file_open,
                                  mock_temporary_chown):
+
         vol = tests_utils.create_volume(self.context)
-        vol['status'] = 'in-use'
+        vol['previous_status'] = 'in-use'
         self.context.user_id = 'fake'
         self.context.project_id = 'fake'
+
+        mock_volume_get.return_value = vol
+        temp_snapshot = tests_utils.create_snapshot(self.context, vol['id'])
         backup = tests_utils.create_backup(self.context,
                                            vol['id'])
         backup_obj = objects.Backup.get_by_id(self.context, backup.id)
@@ -6155,20 +6164,24 @@ class LVMVolumeDriverTestCase(DriverTestCase):
         self.volume.driver._detach_volume = mock.MagicMock()
         self.volume.driver._attach_volume = mock.MagicMock()
         self.volume.driver.terminate_connection = mock.MagicMock()
-        self.volume.driver.create_snapshot = mock.MagicMock()
-        self.volume.driver.delete_snapshot = mock.MagicMock()
+        self.volume.driver._create_temp_snapshot = mock.MagicMock()
+        self.volume.driver._delete_snapshot = mock.MagicMock()
 
-        mock_volume_get.return_value = vol
         mock_get_connector_properties.return_value = properties
         f = mock_file_open.return_value = file('/dev/null')
 
         backup_service.backup(backup_obj, f, None)
         self.volume.driver._attach_volume.return_value = attach_info
+        self.volume.driver._create_temp_snapshot.return_value = temp_snapshot
 
         self.volume.driver.backup_volume(self.context, backup_obj,
                                          backup_service)
 
         mock_volume_get.assert_called_with(self.context, vol['id'])
+        self.volume.driver._create_temp_snapshot.assert_called_once_with(
+            self.context, vol)
+        self.volume.driver._delete_snapshot.assert_called_once_with(
+            self.context, temp_snapshot)
 
     def test_create_volume_from_snapshot_none_sparse(self):
 
diff --git a/cinder/volume/driver.py b/cinder/volume/driver.py
index 2d3bdbc27..71936b2d4 100644
--- a/cinder/volume/driver.py
+++ b/cinder/volume/driver.py
@@ -756,7 +756,7 @@ class BaseVD(object):
         # volume.
         previous_status = volume.get('previous_status', None)
         temp_vol_ref = None
-        if previous_status == "in_use":
+        if previous_status == "in-use":
             temp_vol_ref = self._create_temp_cloned_volume(
                 context, volume)
             backup.temp_volume_id = temp_vol_ref['id']
-- 
2.45.2