From b9ff8cd7ad8c402787324d2baca9b32f61eafb4a Mon Sep 17 00:00:00 2001 From: Eric Harney Date: Mon, 18 Nov 2013 13:42:37 -0500 Subject: [PATCH] LVM: Activate source LV before cloning from it LVM may be configured to not automatically activate thin-provisioned LVs. Ensure they are activated when performing a clone, otherwise dd will fail as the device does not exist in /dev/mapper/. Closes-Bug: #1252423 Change-Id: Ibcb946ffe7804b1976bf1b1863c48340c8cc7fd5 --- cinder/brick/local_dev/lvm.py | 108 +++++++++++++++++++++++++-- cinder/tests/brick/test_brick_lvm.py | 74 ++++++++++++++++++ cinder/volume/drivers/lvm.py | 4 + etc/cinder/rootwrap.d/volume.filters | 3 + 4 files changed, 183 insertions(+), 6 deletions(-) diff --git a/cinder/brick/local_dev/lvm.py b/cinder/brick/local_dev/lvm.py index 47e6d5b9f..872e4fe19 100644 --- a/cinder/brick/local_dev/lvm.py +++ b/cinder/brick/local_dev/lvm.py @@ -64,6 +64,8 @@ class LVM(executor.Executor): self.vg_uuid = None self.vg_thin_pool = None self.vg_thin_pool_size = 0 + self._supports_snapshot_lv_activation = None + self._supports_lvchange_ignoreskipactivation = None if create_vg and physical_volumes is not None: self.pv_list = physical_volumes @@ -120,13 +122,14 @@ class LVM(executor.Executor): return [] @staticmethod - def supports_thin_provisioning(root_helper): - """Static method to check for thin LVM support on a system. + def get_lvm_version(root_helper): + """Static method to get LVM version from system. :param root_helper: root_helper to use for execute - :returns: True if supported, False otherwise + :returns: version 3-tuple """ + cmd = ['vgs', '--version'] (out, err) = putils.execute(*cmd, root_helper=root_helper, @@ -142,9 +145,59 @@ class LVM(executor.Executor): version_filter = "(\d+)\.(\d+)\.(\d+).*" r = re.search(version_filter, version) version_tuple = tuple(map(int, r.group(1, 2, 3))) - if version_tuple >= (2, 2, 95): - return True - return False + return version_tuple + + @staticmethod + def supports_thin_provisioning(root_helper): + """Static method to check for thin LVM support on a system. + + :param root_helper: root_helper to use for execute + :returns: True if supported, False otherwise + + """ + + return LVM.get_lvm_version(root_helper) >= (2, 2, 95) + + @property + def supports_snapshot_lv_activation(self): + """Property indicating whether snap activation changes are supported. + + Check for LVM version > 2.02.91. + (LVM2 git: e8a40f6 Allow to activate snapshot) + + :returns: True/False indicating support + """ + + if self._supports_snapshot_lv_activation is not None: + return self._supports_snapshot_lv_activation + + self._supports_snapshot_lv_activation = ( + self.get_lvm_version(self._root_helper) >= (2, 2, 91)) + + return self._supports_snapshot_lv_activation + + @property + def supports_lvchange_ignoreskipactivation(self): + """Property indicating whether lvchange can ignore skip activation. + + Tests whether lvchange -K/--ignoreactivationskip exists. + """ + + if self._supports_lvchange_ignoreskipactivation is not None: + return self._supports_lvchange_ignoreskipactivation + + cmd = ['lvchange', '--help'] + (out, err) = self._execute(*cmd) + + self._supports_lvchange_ignoreskipactivation = False + + lines = out.split('\n') + for line in lines: + if '-K' in line and '--ignoreactivationskip' in line: + self._supports_lvchange_ignoreskipactivation = True + break + + return self._supports_lvchange_ignoreskipactivation @staticmethod def get_all_volumes(root_helper, vg_name=None, no_suffix=True): @@ -404,6 +457,49 @@ class LVM(executor.Executor): LOG.error(_('StdErr :%s') % err.stderr) raise + def _mangle_lv_name(self, name): + # Linux LVM reserves name that starts with snapshot, so that + # such volume name can't be created. Mangle it. + if not name.startswith('snapshot'): + return name + return '_' + name + + def activate_lv(self, name, is_snapshot=False): + """Ensure that logical volume/snapshot logical volume is activated. + + :param name: Name of LV to activate + :raises: putils.ProcessExecutionError + """ + + # This is a no-op if requested for a snapshot on a version + # of LVM that doesn't support snapshot activation. + # (Assume snapshot LV is always active.) + if is_snapshot and not self.supports_snapshot_lv_activation: + return + + lv_path = self.vg_name + '/' + self._mangle_lv_name(name) + + # Must pass --yes to activate both the snap LV and its origin LV. + # Otherwise lvchange asks if you would like to do this interactively, + # and fails. + cmd = ['lvchange', '-a', 'y', '--yes'] + + if self.supports_lvchange_ignoreskipactivation: + cmd.append('-K') + + cmd.append(lv_path) + + try: + self._execute(*cmd, + root_helper=self._root_helper, + run_as_root=True) + except putils.ProcessExecutionError as err: + LOG.exception(_('Error activating LV')) + LOG.error(_('Cmd :%s') % err.cmd) + LOG.error(_('StdOut :%s') % err.stdout) + LOG.error(_('StdErr :%s') % err.stderr) + raise + def delete(self, name): """Delete logical volume or snapshot. diff --git a/cinder/tests/brick/test_brick_lvm.py b/cinder/tests/brick/test_brick_lvm.py index 83183b59e..9d5218b73 100644 --- a/cinder/tests/brick/test_brick_lvm.py +++ b/cinder/tests/brick/test_brick_lvm.py @@ -56,6 +56,7 @@ class BrickLvmTestCase(test.TestCase): return (" LVM version: 2.03.00 (2012-03-06)\n", "") def fake_old_lvm_version(obj, *cmd, **kwargs): + # Does not support thin prov or snap activation return (" LVM version: 2.02.65(2) (2012-03-06)\n", "") def fake_customised_lvm_version(obj, *cmd, **kwargs): @@ -153,6 +154,65 @@ class BrickLvmTestCase(test.TestCase): self.fake_customised_lvm_version) self.assertTrue(self.vg.supports_thin_provisioning('sudo')) + def test_snapshot_lv_activate_support(self): + self.vg._supports_snapshot_lv_activation = None + self.stubs.Set(processutils, 'execute', self.fake_execute) + self.assertTrue(self.vg.supports_snapshot_lv_activation) + + self.vg._supports_snapshot_lv_activation = None + self.stubs.Set(processutils, 'execute', self.fake_old_lvm_version) + self.assertFalse(self.vg.supports_snapshot_lv_activation) + + self.vg._supports_snapshot_lv_activation = None + + def test_lvchange_ignskipact_support_yes(self): + """Tests the ability to test support for lvchange -K + + Stubs provide output for "lvchange --help". + """ + + def lvchange_ign_yes(obj, *args, **kwargs): + return (""" + WARNING: Running as a non-root user. Functionality may be + unavailable. + lvchange: Change the attributes of logical volume(s) + + lvchange + [-A|--autobackup y|n] + [-k|--setactivationskip {y|n}] + [-K|--ignoreactivationskip] + [-y|--yes] + [-Z|--zero {y|n}] + LogicalVolume[Path] [LogicalVolume[Path]...] + """, "") + + self.vg._supports_lvchange_ignoreskipactivation = None + self.stubs.Set(self.vg, '_execute', lvchange_ign_yes) + self.assertTrue(self.vg.supports_lvchange_ignoreskipactivation) + + self.vg._supports_lvchange_ignoreskipactivation = None + + def test_lvchange_ignskipact_support_no(self): + def lvchange_ign_no(obj, *args, **kwargs): + return (""" + WARNING: Running as a non-root user. Functionality may be + unavailable. + lvchange: Change the attributes of logical volume(s) + + lvchange + [-A|--autobackup y|n] + [-k|--setactivationskip {y|n}] + [-y|--yes] + [-Z|--zero {y|n}] + LogicalVolume[Path] [LogicalVolume[Path]...] + """, "") + + self.vg._supports_lvchange_ignoreskipactivation = None + self.stubs.Set(self.vg, '_execute', lvchange_ign_no) + self.assertFalse(self.vg.supports_lvchange_ignoreskipactivation) + + self.vg._supports_lvchange_ignoreskipactivation = None + def test_volume_create_after_thin_creation(self): """Test self.vg.vg_thin_pool is set to pool_name @@ -175,3 +235,17 @@ class BrickLvmTestCase(test.TestCase): def test_lv_has_snapshot(self): self.assertTrue(self.vg.lv_has_snapshot('fake-volumes')) self.assertFalse(self.vg.lv_has_snapshot('test-volumes')) + + def test_activate_lv(self): + self._mox.StubOutWithMock(self.vg, '_execute') + self.vg._supports_lvchange_ignoreskipactivation = True + + self.vg._execute('lvchange', '-a', 'y', '--yes', '-K', + 'fake-volumes/my-lv', + root_helper='sudo', run_as_root=True) + + self._mox.ReplayAll() + + self.vg.activate_lv('my-lv') + + self._mox.VerifyAll() diff --git a/cinder/volume/drivers/lvm.py b/cinder/volume/drivers/lvm.py index c33532b9e..51d3e628f 100644 --- a/cinder/volume/drivers/lvm.py +++ b/cinder/volume/drivers/lvm.py @@ -166,6 +166,10 @@ class LVMVolumeDriver(driver.VolumeDriver): self.configuration.lvm_type, self.configuration.lvm_mirrors) + # Some configurations of LVM do not automatically activate + # ThinLVM snapshot LVs. + self.vg.activate_lv(snapshot['name'], is_snapshot=True) + volutils.copy_volume(self.local_path(snapshot), self.local_path(volume), snapshot['volume_size'] * 1024, diff --git a/etc/cinder/rootwrap.d/volume.filters b/etc/cinder/rootwrap.d/volume.filters index f9e46d32e..c30aa0894 100644 --- a/etc/cinder/rootwrap.d/volume.filters +++ b/etc/cinder/rootwrap.d/volume.filters @@ -30,6 +30,9 @@ lvrename: CommandFilter, lvrename, root # cinder/volume/driver.py: 'lvextend', '-L' '%(new_size)s', '%(lv_name)s' ... lvextend: CommandFilter, lvextend, root +# cinder/brick/local_dev/lvm.py: 'lvchange -a y -K ' +lvchange: CommandFilter, lvchange, root + # cinder/volume/driver.py: 'iscsiadm', '-m', 'discovery', '-t',... # cinder/volume/driver.py: 'iscsiadm', '-m', 'node', '-T', ... iscsiadm: CommandFilter, iscsiadm, root -- 2.45.2