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