diff --git a/0019-Make-ActionDestroyFormat-optional.patch b/0019-Make-ActionDestroyFormat-optional.patch new file mode 100644 index 0000000..006531d --- /dev/null +++ b/0019-Make-ActionDestroyFormat-optional.patch @@ -0,0 +1,310 @@ +From 8368cab41a1f34452b4c624768245517391ce400 Mon Sep 17 00:00:00 2001 +From: Vojtech Trefny +Date: Fri, 16 May 2025 17:15:17 +0200 +Subject: [PATCH 1/5] Allow ActionDestroyFormat to be marked as optional + +When we are also planning to remove the device, failing to remove +the format is not critical so we can ignore it in these cases. + +Resolves: RHEL-84685 +Resolves: RHEL-84663 +--- + blivet/deviceaction.py | 37 +++++++++++++++++++++++-------------- + 1 file changed, 23 insertions(+), 14 deletions(-) + +diff --git a/blivet/deviceaction.py b/blivet/deviceaction.py +index b22e00c36..2e6a8489f 100644 +--- a/blivet/deviceaction.py ++++ b/blivet/deviceaction.py +@@ -734,12 +734,13 @@ class ActionDestroyFormat(DeviceAction): + obj = ACTION_OBJECT_FORMAT + type_desc_str = N_("destroy format") + +- def __init__(self, device): ++ def __init__(self, device, optional=False): + if device.format_immutable: + raise ValueError("this device's formatting cannot be modified") + + DeviceAction.__init__(self, device) + self.orig_format = self.device.format ++ self.optional = optional + + if not device.format.destroyable: + raise ValueError("resource to destroy this format type %s is unavailable" % device.format.type) +@@ -758,21 +759,29 @@ def execute(self, callbacks=None): + """ wipe the filesystem signature from the device """ + # remove any flag if set + 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: +- if self.format.parted_flag: +- self.device.unset_flag(self.format.parted_flag) +- self.device.disk.original_format.commit_to_disk() +- udev.settle() ++ try: ++ status = self.device.status ++ self.device.setup(orig=True) ++ if hasattr(self.device, 'set_rw'): ++ self.device.set_rw() + +- if not status: +- self.device.teardown() ++ self.format.destroy() ++ udev.settle() ++ if isinstance(self.device, PartitionDevice) and self.device.disklabel_supported: ++ if self.format.parted_flag: ++ self.device.unset_flag(self.format.parted_flag) ++ self.device.disk.original_format.commit_to_disk() ++ udev.settle() ++ ++ if not status: ++ self.device.teardown() ++ except Exception as e: # pylint: disable=broad-except ++ if self.optional: ++ log.error("Ignoring error when executing optional action: Failed to destroy format on %s: %s.", ++ self.device.name, str(e)) ++ else: ++ raise + + def cancel(self): + if not self._applied: + +From 94e0ec7f24129159ac5f4fe455f37b85ceb9a004 Mon Sep 17 00:00:00 2001 +From: Vojtech Trefny +Date: Fri, 16 May 2025 17:28:40 +0200 +Subject: [PATCH 2/5] Make ActionDestroyFormat optional when device is also + removed + +In both destroy_device and recursive_remove we try to remove both +the device and its format. In these cases the format destroy can +be considered to be optional and we don't need to fail just +because we failed to remove the format. + +Resolves: RHEL-84685 +Resolves: RHEL-84663 +--- + blivet/blivet.py | 2 +- + blivet/devicetree.py | 4 ++-- + 2 files changed, 3 insertions(+), 3 deletions(-) + +diff --git a/blivet/blivet.py b/blivet/blivet.py +index 399992a41..53206d973 100644 +--- a/blivet/blivet.py ++++ b/blivet/blivet.py +@@ -915,7 +915,7 @@ def destroy_device(self, device): + if device.format.exists and device.format.type and \ + not device.format_immutable: + # schedule destruction of any formatting while we're at it +- self.devicetree.actions.add(ActionDestroyFormat(device)) ++ self.devicetree.actions.add(ActionDestroyFormat(device, optional=True)) + + action = ActionDestroyDevice(device) + self.devicetree.actions.add(action) +diff --git a/blivet/devicetree.py b/blivet/devicetree.py +index 6a27b1e71..4ec955002 100644 +--- a/blivet/devicetree.py ++++ b/blivet/devicetree.py +@@ -261,7 +261,7 @@ def recursive_remove(self, device, actions=True, remove_device=True, modparent=T + if actions: + if leaf.format.exists and not leaf.protected and \ + not leaf.format_immutable: +- self.actions.add(ActionDestroyFormat(leaf)) ++ self.actions.add(ActionDestroyFormat(leaf, optional=True)) + + self.actions.add(ActionDestroyDevice(leaf)) + else: +@@ -273,7 +273,7 @@ def recursive_remove(self, device, actions=True, remove_device=True, modparent=T + + if not device.format_immutable: + if actions: +- self.actions.add(ActionDestroyFormat(device)) ++ self.actions.add(ActionDestroyFormat(device, optional=True)) + else: + device.format = None + + +From 610b65450fa00a9b8b129ef733536ca080edc6fe Mon Sep 17 00:00:00 2001 +From: Vojtech Trefny +Date: Mon, 19 May 2025 14:24:06 +0200 +Subject: [PATCH 3/5] tests: Add a simple test case for optional format destroy + action + +Related: RHEL-84685 +Related: RHEL-84663 +--- + tests/unit_tests/devices_test/lvm_test.py | 28 +++++++++++++++++++++++ + 1 file changed, 28 insertions(+) + +diff --git a/tests/unit_tests/devices_test/lvm_test.py b/tests/unit_tests/devices_test/lvm_test.py +index ed30772fd..7ec3ed0ae 100644 +--- a/tests/unit_tests/devices_test/lvm_test.py ++++ b/tests/unit_tests/devices_test/lvm_test.py +@@ -1172,3 +1172,31 @@ def test_vdo_compression_deduplication_change(self): + with patch("blivet.devices.lvm.blockdev.lvm") as lvm: + self.b.do_it() + lvm.vdo_enable_deduplication.assert_called_with(vg.name, vdopool.lvname) ++ ++ ++@patch("blivet.devices.lvm.LVMLogicalVolumeDevice._external_dependencies", new=[]) ++@patch("blivet.devices.lvm.LVMLogicalVolumeBase._external_dependencies", new=[]) ++@patch("blivet.devices.dm.DMDevice._external_dependencies", new=[]) ++class BlivetLVMOptionalDestroyTest(BlivetLVMUnitTest): ++ ++ def test_optional_format_destroy(self, *args): # pylint: disable=unused-argument ++ pv = StorageDevice("pv1", fmt=blivet.formats.get_format("lvmpv"), ++ size=Size("10 GiB"), exists=True) ++ vg = LVMVolumeGroupDevice("testvg", parents=[pv], exists=True) ++ lv = LVMLogicalVolumeDevice("testlv", parents=[vg], exists=True, size=Size("5 GiB"), ++ fmt=blivet.formats.get_format("xfs", exists=True)) ++ ++ for dev in (pv, vg, lv): ++ self.b.devicetree._add_device(dev) ++ ++ self.b.destroy_device(lv) ++ fmt_ac = self.b.devicetree.actions.find(action_type="destroy", object_type="format") ++ self.assertTrue(fmt_ac) ++ self.assertTrue(fmt_ac[0].optional) ++ ++ with patch("blivet.devices.lvm.blockdev.lvm") as lvm: ++ lvm.lvactivate.side_effect = RuntimeError() ++ try: ++ self.b.do_it() ++ except RuntimeError: ++ self.fail("Optional format destroy action is not optional") + +From d5c9b690f702d38a9db5bed5d728a1a25fe31077 Mon Sep 17 00:00:00 2001 +From: Vojtech Trefny +Date: Tue, 20 May 2025 13:02:00 +0200 +Subject: [PATCH 4/5] tests: Add test case for removing broken thin pool + +Related: RHEL-84685 +Related: RHEL-84663 +--- + tests/storage_tests/devices_test/lvm_test.py | 51 ++++++++++++++++++++ + 1 file changed, 51 insertions(+) + +diff --git a/tests/storage_tests/devices_test/lvm_test.py b/tests/storage_tests/devices_test/lvm_test.py +index 25d9d71bb..aae9da8b5 100644 +--- a/tests/storage_tests/devices_test/lvm_test.py ++++ b/tests/storage_tests/devices_test/lvm_test.py +@@ -1,6 +1,7 @@ + import os + import shutil + import subprocess ++import tempfile + + from ..storagetestcase import StorageTestCase + +@@ -574,3 +575,53 @@ def test_lvm_pv_size(self): + self.assertEqual(vg_size, vg.size) + vg_free = self._get_vg_free(vg.name) + self.assertEqual(vg_free, vg.free_space) ++ ++ def _break_thin_pool(self): ++ os.system("vgchange -an %s >/dev/null 2>&1" % self.vgname) ++ ++ # changing transaction_id for the pool prevents it from being activated ++ with tempfile.NamedTemporaryFile(prefix="blivet_test") as temp: ++ os.system("vgcfgbackup -f %s %s >/dev/null 2>&1" % (temp.name, self.vgname)) ++ os.system("sed -i 's/transaction_id =.*/transaction_id = 123456/' %s >/dev/null 2>&1" % temp.name) ++ os.system("vgcfgrestore -f %s %s --force >/dev/null 2>&1" % (temp.name, self.vgname)) ++ ++ def test_lvm_broken_thin(self): ++ disk = self.storage.devicetree.get_device_by_path(self.vdevs[0]) ++ self.assertIsNotNone(disk) ++ ++ self.storage.initialize_disk(disk) ++ ++ pv = self.storage.new_partition(size=blivet.size.Size("100 MiB"), fmt_type="lvmpv", ++ parents=[disk]) ++ self.storage.create_device(pv) ++ ++ blivet.partitioning.do_partitioning(self.storage) ++ ++ vg = self.storage.new_vg(name=self.vgname, parents=[pv]) ++ self.storage.create_device(vg) ++ ++ pool = self.storage.new_lv(thin_pool=True, size=blivet.size.Size("50 MiB"), ++ parents=[vg], name="blivetTestPool") ++ self.storage.create_device(pool) ++ ++ self.storage.do_it() ++ ++ # intentionally break the thin pool created above ++ self._break_thin_pool() ++ ++ self.storage.reset() ++ ++ pool = self.storage.devicetree.get_device_by_name("%s-blivetTestPool" % self.vgname) ++ self.assertIsNotNone(pool) ++ ++ # check that the pool cannot be activated ++ try: ++ pool.setup() ++ except Exception: # pylint: disable=broad-except ++ pass ++ else: ++ self.fail("Failed to break thinpool for tests") ++ ++ # verify that the pool can be destroyed even if it cannot be activated ++ self.storage.recursive_remove(pool) ++ self.storage.do_it() + +From 6f0625e06a2ea69be8042cf5e76048b97a1025e1 Mon Sep 17 00:00:00 2001 +From: Vojtech Trefny +Date: Tue, 29 Apr 2025 08:09:06 +0200 +Subject: [PATCH 5/5] Fix expected exception type when activating devices in + populor + +We are no longer raising libblockdev exceptions in our public API +calls (see #1014) so when calling setup() ourselves we need to +catch our exceptions instead of libblockdev ones as well. + +Related: RHEL-84685 +Related: RHEL-84663 +--- + blivet/populator/helpers/luks.py | 2 +- + blivet/populator/helpers/lvm.py | 4 ++-- + 2 files changed, 3 insertions(+), 3 deletions(-) + +diff --git a/blivet/populator/helpers/luks.py b/blivet/populator/helpers/luks.py +index 72da248ed..0b72920e3 100644 +--- a/blivet/populator/helpers/luks.py ++++ b/blivet/populator/helpers/luks.py +@@ -161,7 +161,7 @@ def run(self): + self.device.format.passphrase = passphrase + try: + self.device.format.setup() +- except blockdev.BlockDevError: ++ except LUKSError: + self.device.format.passphrase = None + else: + break +diff --git a/blivet/populator/helpers/lvm.py b/blivet/populator/helpers/lvm.py +index e22c52088..cdf97e405 100644 +--- a/blivet/populator/helpers/lvm.py ++++ b/blivet/populator/helpers/lvm.py +@@ -29,7 +29,7 @@ + from ... import udev + from ...devicelibs import lvm + from ...devices.lvm import LVMVolumeGroupDevice, LVMLogicalVolumeDevice, LVMInternalLVtype +-from ...errors import DeviceTreeError, DuplicateVGError ++from ...errors import DeviceTreeError, DuplicateVGError, LVMError + from ...flags import flags + from ...size import Size + from ...storage_log import log_method_call +@@ -289,7 +289,7 @@ def add_lv(lv): + if flags.auto_dev_updates: + try: + lv_device.setup() +- except blockdev.LVMError: ++ except LVMError: + log.warning("failed to activate lv %s", lv_device.name) + lv_device.controllable = False + diff --git a/python-blivet.spec b/python-blivet.spec index f1c1b97..fc393ba 100644 --- a/python-blivet.spec +++ b/python-blivet.spec @@ -5,7 +5,7 @@ Version: 3.10.0 #%%global prerelease .b2 # prerelease, if defined, should be something like .a1, .b1, .b2.dev1, or .c2 -Release: 18%{?prerelease}%{?dist} +Release: 19%{?prerelease}%{?dist} Epoch: 1 License: LGPL-2.1-or-later %global realname blivet @@ -33,6 +33,7 @@ Patch13: 0015-iscsi-Use-node-startup-onboot-option-for-Login.patch Patch14: 0016-Make-sure-selinux_test-doesnt-try-to-create-mountpoints.patch Patch15: 0017-LVMPV-format-size-fix.patch Patch16: 0018-Include-additional-information-in-PartitioningError.patch +Patch17: 0019-Make-ActionDestroyFormat-optional.patch # Versions of required components (done so we make sure the buildrequires # match the requires versions of things). @@ -127,6 +128,11 @@ make DESTDIR=%{buildroot} install %{python3_sitelib}/* %changelog +* Tue May 20 2025 Vojtech Trefny - 3.10.0-19 +- Make ActionDestroyFormat optional when the device is also scheduled to be removed + Resolves: RHEL-84685 + Resolves: RHEL-84663 + * Mon Apr 14 2025 Vojtech Trefny - 3.10.0-18 - Get the actual PV format size for LVMPV format Resolves: RHEL-74076