From 5e0957b20d6ada212021aad3adb2d248154b4b67 Mon Sep 17 00:00:00 2001
From: Huangsm <huangsm@awcloud.com>
Date: Tue, 14 Feb 2017 18:09:31 +0800
Subject: [PATCH] Correct Extend lvm Volume

A volume extened which has some snapshot will be deactivated.
With this change, the volume will be reactivated.

Change-Id: I11c9b5935d74a5a3d4562ec1ce534b9e412d6030
Closes-Bug: #1663525
---
 cinder/brick/local_dev/lvm.py             |  5 +++-
 cinder/tests/unit/brick/test_brick_lvm.py | 29 +++++++++++++----------
 2 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/cinder/brick/local_dev/lvm.py b/cinder/brick/local_dev/lvm.py
index dbe7667d217..8000f5c4043 100644
--- a/cinder/brick/local_dev/lvm.py
+++ b/cinder/brick/local_dev/lvm.py
@@ -805,7 +805,8 @@ class LVM(executor.Executor):
         # deactivated, but Thin Volumes with snaps have attribute 'V'
         # and won't be deactivated because the lv_has_snapshot method looks
         # for 'o' or 'O'
-        if self.lv_has_snapshot(lv_name):
+        has_snapshot = self.lv_has_snapshot(lv_name)
+        if has_snapshot:
             self.deactivate_lv(lv_name)
         try:
             cmd = LVM.LVM_CMD_PREFIX + ['lvextend', '-L', new_size,
@@ -818,6 +819,8 @@ class LVM(executor.Executor):
             LOG.error(_LE('StdOut  :%s'), err.stdout)
             LOG.error(_LE('StdErr  :%s'), err.stderr)
             raise
+        if has_snapshot:
+            self.activate_lv(lv_name)
 
     def vg_mirror_free_space(self, mirror_count):
         free_capacity = 0.0
diff --git a/cinder/tests/unit/brick/test_brick_lvm.py b/cinder/tests/unit/brick/test_brick_lvm.py
index 54aa1020dc2..8a102bbdca3 100644
--- a/cinder/tests/unit/brick/test_brick_lvm.py
+++ b/cinder/tests/unit/brick/test_brick_lvm.py
@@ -12,6 +12,7 @@
 #    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
 #    License for the specific language governing permissions and limitations
 #    under the License.
+import ddt
 import mock
 from oslo_concurrency import processutils
 
@@ -21,6 +22,7 @@ from cinder import test
 from cinder.volume import configuration as conf
 
 
+@ddt.ddt
 class BrickLvmTestCase(test.TestCase):
     def setUp(self):
         if not hasattr(self, 'configuration'):
@@ -389,20 +391,23 @@ class BrickLvmTestCase(test.TestCase):
     def test_get_mirrored_available_capacity(self):
         self.assertEqual(2.0, self.vg.vg_mirror_free_space(1))
 
-    def test_lv_extend(self):
-        self.vg.deactivate_lv = mock.MagicMock()
+    @ddt.data(True, False)
+    def test_lv_extend(self, has_snapshot):
+        with mock.patch.object(self.vg, '_execute'):
+            with mock.patch.object(self.vg, 'lv_has_snapshot'):
+                self.vg.deactivate_lv = mock.MagicMock()
+                self.vg.activate_lv = mock.MagicMock()
 
-        # Extend lv with snapshot and make sure deactivate called
-        self.vg.create_volume("test", "1G")
-        self.vg.extend_volume("test", "2G")
-        self.vg.deactivate_lv.assert_called_once_with('test')
-        self.vg.deactivate_lv.reset_mock()
+                self.vg.lv_has_snapshot.return_value = has_snapshot
+                self.vg.extend_volume("test", "2G")
 
-        # Extend lv without snapshot so deactivate should not be called
-        self.vg.create_volume("test", "1G")
-        self.vg.vg_name = "test-volumes"
-        self.vg.extend_volume("test", "2G")
-        self.assertFalse(self.vg.deactivate_lv.called)
+                self.vg.lv_has_snapshot.assert_called_once_with("test")
+                if has_snapshot:
+                    self.vg.activate_lv.assert_called_once_with("test")
+                    self.vg.deactivate_lv.assert_called_once_with("test")
+                else:
+                    self.vg.activate_lv.assert_not_called()
+                    self.vg.deactivate_lv.assert_not_called()
 
     def test_lv_deactivate(self):
         with mock.patch.object(self.vg, '_execute'):