From 314611f79afa6fe79f3d5022c814772048669bb0 Mon Sep 17 00:00:00 2001 From: Curt Bruns Date: Mon, 6 Jul 2015 15:09:41 +0000 Subject: [PATCH] Add deactivate step to extend_lv Extending a linear LVM volume that has a snapshot requires that the LV be deactivated explicitly prior to extending. This adds a deactivate_lv method and calls it prior to issuing the extend call. If auto_activation_volume_list is not defined lvm.conf, then volumes will be reactivated automatically after the extend. If auto_activation_volume_list is defined, then volumes that should be automatically reactivated should be added to the auto_activation_volume_list or they won't be activated automatically. NOTE: This doesn't apply to thin provisioned LVs as they don't require the deactivation step. DocImpact: When a user extends LVM Volumes with a snapshot, the volumes will be deactivated. Their re-activation is automatic, unless auto_activation_volume_list is defined in lvm.conf. See lvm.conf for more info. Thin provisioned LVM Volumes will not be deactivated as they don't require it. Co-Authored-By: John Griffith Change-Id: If746625cfe658c3528525dc7f4bf869f1c9704dc Closes-Bug: #1470558 --- cinder/brick/local_dev/lvm.py | 22 +++++++++++++++++++++- cinder/tests/unit/brick/test_brick_lvm.py | 20 ++++++++++++++++++++ cinder/tests/unit/test_srb.py | 3 ++- 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/cinder/brick/local_dev/lvm.py b/cinder/brick/local_dev/lvm.py index 83524856f..b830dd702 100644 --- a/cinder/brick/local_dev/lvm.py +++ b/cinder/brick/local_dev/lvm.py @@ -589,6 +589,21 @@ class LVM(executor.Executor): return name return '_' + name + def deactivate_lv(self, name): + lv_path = self.vg_name + '/' + self._mangle_lv_name(name) + cmd = ['lvchange', '-a', 'n'] + cmd.append(lv_path) + try: + self._execute(*cmd, + root_helper=self._root_helper, + run_as_root=True) + except putils.ProcessExecutionError as err: + LOG.exception(_LE('Error deactivating LV')) + LOG.error(_LE('Cmd :%s'), err.cmd) + LOG.error(_LE('StdOut :%s'), err.stdout) + LOG.error(_LE('StdErr :%s'), err.stderr) + raise + def activate_lv(self, name, is_snapshot=False): """Ensure that logical volume/snapshot logical volume is activated. @@ -696,7 +711,12 @@ class LVM(executor.Executor): def extend_volume(self, lv_name, new_size): """Extend the size of an existing volume.""" - + # Volumes with snaps have attributes 'o' or 'O' and will be + # 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): + self.deactivate_lv(lv_name) try: self._execute('lvextend', '-L', new_size, '%s/%s' % (self.vg_name, lv_name), diff --git a/cinder/tests/unit/brick/test_brick_lvm.py b/cinder/tests/unit/brick/test_brick_lvm.py index 77744c688..ab2760091 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 mock from mox3 import mox from oslo_concurrency import processutils @@ -140,8 +141,12 @@ class BrickLvmTestCase(test.TestCase): pass elif 'lvcreate, -T, -V, ' in cmd_string: pass + elif 'lvcreate, -n, ' in cmd_string: + pass elif 'lvcreate, --name, ' in cmd_string: pass + elif 'lvextend, -L, ' in cmd_string: + pass else: raise AssertionError('unexpected command called: %s' % cmd_string) @@ -351,3 +356,18 @@ class BrickLvmTestCase(test.TestCase): def test_get_mirrored_available_capacity(self): self.assertEqual(self.vg.vg_mirror_free_space(1), 2.0) + + def test_lv_extend(self): + self.vg.deactivate_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() + + # 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) diff --git a/cinder/tests/unit/test_srb.py b/cinder/tests/unit/test_srb.py index 329b8a65a..b0100c15f 100644 --- a/cinder/tests/unit/test_srb.py +++ b/cinder/tests/unit/test_srb.py @@ -593,7 +593,8 @@ class SRBDriverTestCase(test.TestCase): def _fake_lvchange(self): def check(cmd_string): - return 'lvchange, -a, y, --yes' in cmd_string + return 'lvchange, -a, y, --yes' in cmd_string or \ + 'lvchange, -a, n' in cmd_string def act(_): pass -- 2.45.2