From c85a80ca54eabb1cf2458a3e17b3472ba2eb0914 Mon Sep 17 00:00:00 2001
From: David Lehman <dlehman@redhat.com>
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 <dlehman@redhat.com>
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