From c85a80ca54eabb1cf2458a3e17b3472ba2eb0914 Mon Sep 17 00:00:00 2001 From: David Lehman Date: Fri, 1 Nov 2019 12:07:43 -0400 Subject: [PATCH 1/2] Override LVM skip-activation to allow for thorough removal. When we have been told to remove the LV or manage the formatting we must tell LVM to ignore the skip-activation bit. Otherwise we have no way to properly perform the requested management. Resolves: rhbz#1766498 --- blivet/deviceaction.py | 35 ++++++++++++++++++++++++++++++++++ blivet/devices/lvm.py | 12 ++++-------- tests/action_test.py | 16 ++++++++++++++++ tests/devices_test/lvm_test.py | 29 ++++++++++++++++++++++++++++ 4 files changed, 84 insertions(+), 8 deletions(-) diff --git a/blivet/deviceaction.py b/blivet/deviceaction.py index 14a06ff0..57115662 100644 --- a/blivet/deviceaction.py +++ b/blivet/deviceaction.py @@ -393,10 +393,29 @@ class ActionDestroyDevice(DeviceAction): super(ActionDestroyDevice, self)._check_device_dependencies() + def apply(self): + """ apply changes related to the action to the device(s) """ + if self._applied: + return + + if hasattr(self.device, 'ignore_skip_activation'): + self.device.ignore_skip_activation += 1 + + super(ActionDestroyDevice, self).apply() + def execute(self, callbacks=None): super(ActionDestroyDevice, self).execute(callbacks=callbacks) self.device.destroy() + def cancel(self): + if not self._applied: + return + + if hasattr(self.device, 'ignore_skip_activation'): + self.device.ignore_skip_activation -= 1 + + super(ActionDestroyDevice, self).cancel() + def requires(self, action): """ Return True if self requires action. @@ -715,6 +734,9 @@ class ActionDestroyFormat(DeviceAction): return self.device.format = None + if hasattr(self.device, 'ignore_skip_activation'): + self.device.ignore_skip_activation += 1 + super(ActionDestroyFormat, self).apply() def execute(self, callbacks=None): @@ -739,6 +761,8 @@ class ActionDestroyFormat(DeviceAction): return self.device.format = self.orig_format + if hasattr(self.device, 'ignore_skip_activation'): + self.device.ignore_skip_activation -= 1 super(ActionDestroyFormat, self).cancel() @property @@ -834,6 +858,9 @@ class ActionResizeFormat(DeviceAction): return self.device.format.target_size = self._target_size + if hasattr(self.device, 'ignore_skip_activation'): + self.device.ignore_skip_activation += 1 + super(ActionResizeFormat, self).apply() def execute(self, callbacks=None): @@ -854,6 +881,9 @@ class ActionResizeFormat(DeviceAction): return self.device.format.target_size = self.orig_size + if hasattr(self.device, 'ignore_skip_activation'): + self.device.ignore_skip_activation -= 1 + super(ActionResizeFormat, self).cancel() def requires(self, action): @@ -1056,6 +1086,9 @@ class ActionConfigureFormat(DeviceAction): return setattr(self.device.format, self.attr, self.new_value) + if hasattr(self.device, 'ignore_skip_activation'): + self.device.ignore_skip_activation += 1 + super(ActionConfigureFormat, self).apply() def cancel(self): @@ -1063,6 +1096,8 @@ class ActionConfigureFormat(DeviceAction): return setattr(self.device.format, self.attr, self.old_value) + if hasattr(self.device, 'ignore_skip_activation'): + self.device.ignore_skip_activation -= 1 def execute(self, callbacks=None): super(ActionConfigureFormat, self).execute(callbacks=callbacks) diff --git a/blivet/devices/lvm.py b/blivet/devices/lvm.py index 06191110..58adf5cf 100644 --- a/blivet/devices/lvm.py +++ b/blivet/devices/lvm.py @@ -628,6 +628,8 @@ class LVMLogicalVolumeBase(DMDevice, RaidDevice): self.uuid = uuid self.seg_type = seg_type or "linear" self._raid_level = None + self.ignore_skip_activation = 0 + if self.seg_type in lvm.raid_seg_types: self._raid_level = lvm.raid_levels.raid_level(self.seg_type) else: @@ -1367,12 +1369,6 @@ class LVMSnapshotMixin(object): # the old snapshot cannot be setup and torn down pass - def _setup(self, orig=False): - """ Open, or set up, a device. """ - log_method_call(self, self.name, orig=orig, status=self.status, - controllable=self.controllable) - blockdev.lvm.lvactivate(self.vg.name, self._name, ignore_skip=True) - @old_snapshot_specific def teardown(self, recursive=False): # the old snapshot cannot be setup and torn down @@ -1969,12 +1965,12 @@ class LVMLogicalVolumeDevice(LVMLogicalVolumeBase, LVMInternalLogicalVolumeMixin def display_lv_name(self): return self.lvname - @type_specific def _setup(self, orig=False): """ Open, or set up, a device. """ log_method_call(self, self.name, orig=orig, status=self.status, controllable=self.controllable) - blockdev.lvm.lvactivate(self.vg.name, self._name) + ignore_skip_activation = self.is_snapshot_lv or self.ignore_skip_activation > 0 + blockdev.lvm.lvactivate(self.vg.name, self._name, ignore_skip=ignore_skip_activation) @type_specific def _pre_create(self): diff --git a/tests/action_test.py b/tests/action_test.py index 101d5a21..24ed10b2 100644 --- a/tests/action_test.py +++ b/tests/action_test.py @@ -1025,12 +1025,28 @@ class DeviceActionTestCase(StorageTestCase): # ActionDestroyFormat original_format = lv_root.format action = ActionDestroyFormat(lv_root) + orig_ignore_skip = lv_root.ignore_skip_activation self.assertEqual(lv_root.format, original_format) self.assertNotEqual(lv_root.format.type, None) action.apply() self.assertEqual(lv_root.format.type, None) + self.assertEqual(lv_root.ignore_skip_activation, orig_ignore_skip + 1) action.cancel() self.assertEqual(lv_root.format, original_format) + self.assertEqual(lv_root.ignore_skip_activation, orig_ignore_skip) + + # ActionDestroyDevice + action1 = ActionDestroyFormat(lv_root) + orig_ignore_skip = lv_root.ignore_skip_activation + action1.apply() + self.assertEqual(lv_root.ignore_skip_activation, orig_ignore_skip + 1) + action2 = ActionDestroyDevice(lv_root) + action2.apply() + self.assertEqual(lv_root.ignore_skip_activation, orig_ignore_skip + 2) + action2.cancel() + self.assertEqual(lv_root.ignore_skip_activation, orig_ignore_skip + 1) + action1.cancel() + self.assertEqual(lv_root.ignore_skip_activation, orig_ignore_skip) sdc = self.storage.devicetree.get_device_by_name("sdc") sdc.format = None diff --git a/tests/devices_test/lvm_test.py b/tests/devices_test/lvm_test.py index 76a3a5db..c4c50748 100644 --- a/tests/devices_test/lvm_test.py +++ b/tests/devices_test/lvm_test.py @@ -360,6 +360,35 @@ class LVMDeviceTest(unittest.TestCase): with six.assertRaisesRegex(self, ValueError, "The volume group testvg cannot be created."): LVMVolumeGroupDevice("testvg", parents=[pv, pv2]) + def test_skip_activate(self): + pv = StorageDevice("pv1", fmt=blivet.formats.get_format("lvmpv"), + size=Size("1 GiB"), exists=True) + vg = LVMVolumeGroupDevice("testvg", parents=[pv], exists=True) + lv = LVMLogicalVolumeDevice("data_lv", parents=[vg], size=Size("500 MiB"), exists=True) + + with patch("blivet.devices.lvm.blockdev.lvm") as lvm: + with patch.object(lv, "_pre_setup"): + lv.setup() + self.assertTrue(lvm.lvactivate.called_with(vg.name, lv.lvname, ignore_skip=False)) + + lv.ignore_skip_activation += 1 + with patch("blivet.devices.lvm.blockdev.lvm") as lvm: + with patch.object(lv, "_pre_setup"): + lv.setup() + self.assertTrue(lvm.lvactivate.called_with(vg.name, lv.lvname, ignore_skip=True)) + + lv.ignore_skip_activation += 1 + with patch("blivet.devices.lvm.blockdev.lvm") as lvm: + with patch.object(lv, "_pre_setup"): + lv.setup() + self.assertTrue(lvm.lvactivate.called_with(vg.name, lv.lvname, ignore_skip=True)) + + lv.ignore_skip_activation -= 2 + with patch("blivet.devices.lvm.blockdev.lvm") as lvm: + with patch.object(lv, "_pre_setup"): + lv.setup() + self.assertTrue(lvm.lvactivate.called_with(vg.name, lv.lvname, ignore_skip=False)) + class TypeSpecificCallsTest(unittest.TestCase): def test_type_specific_calls(self): -- 2.24.1 From 0e19f91ff0917b7c498cdc2e6d5484847cf18cee Mon Sep 17 00:00:00 2001 From: David Lehman Date: Tue, 17 Dec 2019 14:43:02 -0500 Subject: [PATCH 2/2] Make sure LVs are writable before wiping. Related: rhbz#1766498 --- blivet/deviceaction.py | 3 +++ blivet/devicelibs/lvm.py | 18 ++++++++++++++++++ blivet/devices/lvm.py | 4 ++++ 3 files changed, 25 insertions(+) diff --git a/blivet/deviceaction.py b/blivet/deviceaction.py index 57115662..ac89365b 100644 --- a/blivet/deviceaction.py +++ b/blivet/deviceaction.py @@ -745,6 +745,9 @@ class ActionDestroyFormat(DeviceAction): super(ActionDestroyFormat, self).execute(callbacks=callbacks) status = self.device.status self.device.setup(orig=True) + if hasattr(self.device, 'set_rw'): + self.device.set_rw() + self.format.destroy() udev.settle() if isinstance(self.device, PartitionDevice) and self.device.disklabel_supported: diff --git a/blivet/devicelibs/lvm.py b/blivet/devicelibs/lvm.py index 8eea9d19..65dc425e 100644 --- a/blivet/devicelibs/lvm.py +++ b/blivet/devicelibs/lvm.py @@ -38,7 +38,9 @@ from . import raid from ..size import Size from ..i18n import N_ from ..flags import flags +from ..static_data import lvs_info from ..tasks import availability +from ..util import run_program # some of lvm's defaults that we have no way to ask it for LVM_PE_START = Size("1 MiB") @@ -187,6 +189,22 @@ def lvmetad_socket_exists(): return os.path.exists(LVMETAD_SOCKET_PATH) +def ensure_lv_is_writable(vg_name, lv_name): + lv_info = lvs_info.cache.get("%s-%s" % (vg_name, lv_name)) + if lv_info is None: + return + + if lv_info.attr[1] == 'w': + return + + try: + rc = run_program(['lvchange', '-prw', "%s/%s" % (vg_name, lv_name)]) + except OSError: + rc = -1 + + return rc == 0 + + def is_lvm_name_valid(name): # No . or .. if name == '.' or name == '..': diff --git a/blivet/devices/lvm.py b/blivet/devices/lvm.py index 58adf5cf..dbecc1e5 100644 --- a/blivet/devices/lvm.py +++ b/blivet/devices/lvm.py @@ -951,6 +951,10 @@ class LVMLogicalVolumeBase(DMDevice, RaidDevice): # set up the vg's pvs so lvm can remove the lv self.vg.setup_parents(orig=True) + def set_rw(self): + """ Run lvchange as needed to ensure the lv is not read-only. """ + lvm.ensure_lv_is_writable(self.vg.name, self.lvname) + @property def lvname(self): """ The LV's name (not including VG name). """ -- 2.24.1