Merge "Correct Extend lvm Volume"

cinder/brick/local_dev/lvm.py

@@ -804,7 +804,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,
@@ -817,6 +818,8 @@ class LVM(executor.Executor):
             LOG.error('StdOut  :%s', err.stdout)
             LOG.error('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

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'):