rpms/python-blivet
6
0
Fork 1
Code Issues Pull Requests Releases Activity
dec18aba4e
python-blivet/0036-Make-ActionDestroyFormat-optional.patch
Vojtech Trefny dec18aba4e Make ActionDestroyFormat optional when the device is also scheduled to be removed 
Resolves: RHEL-8008
Resolves: RHEL-8012
2025-05-20 16:06:31 +02:00

259 lines
10 KiB
Diff
Raw Blame History

From 68db0569b3508bbedf33d9ee3b69e8fc6a309b65 Mon Sep 17 00:00:00 2001
From: Vojtech Trefny <vtrefny@redhat.com>
Date: Fri, 16 May 2025 17:15:17 +0200
Subject: [PATCH 1/4] 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-8008
Resolves: RHEL-8012
---
blivet/deviceaction.py | 37 +++++++++++++++++++++++--------------
1 file changed, 23 insertions(+), 14 deletions(-)
diff --git a/blivet/deviceaction.py b/blivet/deviceaction.py
index fc1ca4b65..a6fc211ea 100644
--- a/blivet/deviceaction.py
+++ b/blivet/deviceaction.py
@@ -728,12 +728,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)
@@ -752,21 +753,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 fca71515094840ab1ca8821641284cfb0b687d82 Mon Sep 17 00:00:00 2001
From: Vojtech Trefny <vtrefny@redhat.com>
Date: Fri, 16 May 2025 17:28:40 +0200
Subject: [PATCH 2/4] 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-8008
Resolves: RHEL-8012
---
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 dc066b036..2e86f5bf6 100644
--- a/blivet/blivet.py
+++ b/blivet/blivet.py
@@ -897,7 +897,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 c6c1b4400..f94e3ca30 100644
--- a/blivet/devicetree.py
+++ b/blivet/devicetree.py
@@ -264,7 +264,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:
@@ -276,7 +276,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 50efc63fa3053f863d03439a507b3e0a6d7b8168 Mon Sep 17 00:00:00 2001
From: Vojtech Trefny <vtrefny@redhat.com>
Date: Mon, 19 May 2025 14:24:06 +0200
Subject: [PATCH 3/4] tests: Add a simple test case for optional format destroy
action
Related: RHEL-8008
Related: RHEL-8012
---
tests/unit_tests/devices_test/lvm_test.py | 29 +++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/tests/unit_tests/devices_test/lvm_test.py b/tests/unit_tests/devices_test/lvm_test.py
index e645309fc..34c2084a8 100644
--- a/tests/unit_tests/devices_test/lvm_test.py
+++ b/tests/unit_tests/devices_test/lvm_test.py
@@ -1160,3 +1160,32 @@ def test_vdo_compression_deduplication_change(self):
with patch("blivet.devices.lvm.blockdev.lvm") as lvm:
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(unittest.TestCase):
+
+ def test_optional_format_destroy(self, *args): # pylint: disable=unused-argument
+ b = blivet.Blivet()
+ 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):
+ b.devicetree._add_device(dev)
+
+ b.destroy_device(lv)
+ fmt_ac = 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:
+ b.do_it()
+ except RuntimeError:
+ self.fail("Optional format destroy action is not optional")
From ea913c5fa8e60cd5c2fdd8196be51c067a2a73d8 Mon Sep 17 00:00:00 2001
From: Vojtech Trefny <vtrefny@redhat.com>
Date: Tue, 20 May 2025 13:02:00 +0200
Subject: [PATCH 4/4] tests: Add test case for removing broken thin pool
Related: RHEL-8008
Related: RHEL-8012
---
tests/storage_tests/devices_test/lvm_test.py | 52 ++++++++++++++++++++
1 file changed, 52 insertions(+)
diff --git a/tests/storage_tests/devices_test/lvm_test.py b/tests/storage_tests/devices_test/lvm_test.py
index a1064c9c4..10e7354ff 100644
--- a/tests/storage_tests/devices_test/lvm_test.py
+++ b/tests/storage_tests/devices_test/lvm_test.py
@@ -1,5 +1,7 @@
import os
import subprocess
+import tempfile
+from unittest.mock import patch
from ..storagetestcase import StorageTestCase
@@ -552,3 +554,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, vgname):
+ os.system("vgchange -an %s >/dev/null 2>&1" % 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, 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, 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="blivetTestVG", 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("blivetTestVG")
+
+ self.storage.reset()
+
+ pool = self.storage.devicetree.get_device_by_name("blivetTestVG-blivetTestPool")
+ 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()
Reference in New Issue View Git Blame Copy Permalink