diff --git a/blivet/devicetree.py b/blivet/devicetree.py index ddc94813e..cec34a78b 100644 --- a/blivet/devicetree.py +++ b/blivet/devicetree.py @@ -39,6 +39,7 @@ from . import formats from .devicelibs import lvm from .events.handler import EventHandlerMixin +from .flags import flags from . import util from .populator import PopulatorMixin from .storage_log import log_method_call, log_method_return @@ -133,8 +134,9 @@ def devices(self): continue if device.uuid and device.uuid in [d.uuid for d in devices] and \ + not flags.allow_inconsistent_config and \ not isinstance(device, NoDevice): - raise DeviceTreeError("duplicate uuids in device tree") + raise DuplicateUUIDError("duplicate uuids in device tree") devices.append(device) @@ -174,7 +176,7 @@ def _add_device(self, newdev, new=True): dev = self.get_device_by_uuid(newdev.uuid, incomplete=True, hidden=True) if dev.name == newdev.name: raise DeviceTreeError("Trying to add already existing device.") - else: + elif not flags.allow_inconsistent_config: raise DuplicateUUIDError("Duplicate UUID '%s' found for devices: " "'%s' and '%s'." % (newdev.uuid, newdev.name, dev.name)) @@ -515,7 +517,14 @@ def get_device_by_uuid(self, uuid, incomplete=False, hidden=False): result = None if uuid: devices = self._filter_devices(incomplete=incomplete, hidden=hidden) - result = next((d for d in devices if d.uuid == uuid or d.format.uuid == uuid), None) + matches = [d for d in devices if d.uuid == uuid or d.format.uuid == uuid] + if len(matches) > 1: + log.error("found non-unique UUID %s: %s", uuid, [m.name for m in matches]) + raise DuplicateUUIDError("Duplicate UUID '%s' found for devices: %s" + % (uuid, [m.name for m in matches])) + elif matches: + result = matches[0] + log_method_return(self, result) return result diff --git a/blivet/errors.py b/blivet/errors.py index 781ec83db..d3e87f624 100644 --- a/blivet/errors.py +++ b/blivet/errors.py @@ -224,8 +224,7 @@ class DeviceNotFoundError(StorageError): pass -class UnusableConfigurationError(StorageError): - +class UnusableConfigurationError(DeviceTreeError, StorageError): """ User has an unusable initial storage configuration. """ suggestion = "" diff --git a/blivet/flags.py b/blivet/flags.py index a11ed5c8e..82c731464 100644 --- a/blivet/flags.py +++ b/blivet/flags.py @@ -82,6 +82,9 @@ def __init__(self): # compression option for btrfs filesystems self.btrfs_compression = None + # allow duplicate UUIDs in the devicetree + self.allow_inconsistent_config = True + self.debug_threads = False # Assign GPT partition type UUIDs to allow partition diff --git a/blivet/populator/helpers/disk.py b/blivet/populator/helpers/disk.py index 8936d4de2..659ef4332 100644 --- a/blivet/populator/helpers/disk.py +++ b/blivet/populator/helpers/disk.py @@ -36,6 +36,7 @@ from ...devices import NVMeNamespaceDevice, NVMeFabricsNamespaceDevice from ...devices import device_path_to_name from ...storage_log import log_method_call +from ...tasks import availability from .devicepopulator import DevicePopulator import logging @@ -151,6 +152,7 @@ class MDBiosRaidDevicePopulator(DiskDevicePopulator): _device_class = MDBiosRaidArrayDevice @classmethod + @availability.blockdev_md_required() def match(cls, data): return (super(MDBiosRaidDevicePopulator, MDBiosRaidDevicePopulator).match(data) and udev.device_get_md_container(data)) diff --git a/blivet/populator/helpers/dm.py b/blivet/populator/helpers/dm.py index 7dc15db98..908157d8f 100644 --- a/blivet/populator/helpers/dm.py +++ b/blivet/populator/helpers/dm.py @@ -23,6 +23,7 @@ from ... import udev from ...devices import DMDevice from ...storage_log import log_method_call +from ...tasks import availability from .devicepopulator import DevicePopulator import logging @@ -33,6 +34,7 @@ class DMDevicePopulator(DevicePopulator): priority = 50 @classmethod + @availability.blockdev_dm_required() def match(cls, data): return (udev.device_is_dm(data) and not udev.device_is_dm_partition(data) and diff --git a/blivet/populator/helpers/loop.py b/blivet/populator/helpers/loop.py index 956b0a52b..a084f9380 100644 --- a/blivet/populator/helpers/loop.py +++ b/blivet/populator/helpers/loop.py @@ -27,11 +27,13 @@ from ... import udev from ...devices import FileDevice, LoopDevice from ...storage_log import log_method_call +from ...tasks import availability from .devicepopulator import DevicePopulator class LoopDevicePopulator(DevicePopulator): @classmethod + @availability.blockdev_loop_required() def match(cls, data): return udev.device_is_loop(data) diff --git a/blivet/populator/helpers/lvm.py b/blivet/populator/helpers/lvm.py index 4ba51e4b8..98737fac5 100644 --- a/blivet/populator/helpers/lvm.py +++ b/blivet/populator/helpers/lvm.py @@ -33,7 +33,7 @@ from ...flags import flags from ...size import Size from ...storage_log import log_method_call -from ...tasks.availability import BLOCKDEV_LVM_PLUGIN_VDO +from ...tasks import availability from .devicepopulator import DevicePopulator from .formatpopulator import FormatPopulator @@ -45,6 +45,7 @@ class LVMDevicePopulator(DevicePopulator): @classmethod + @availability.blockdev_lvm_required() def match(cls, data): return udev.device_is_dm_lvm(data) @@ -87,6 +88,11 @@ class LVMFormatPopulator(FormatPopulator): priority = 100 _type_specifier = "lvmpv" + @classmethod + @availability.blockdev_lvm_required() + def match(cls, data, device): + return super(cls, cls).match(data, device) + def _get_kwargs(self): kwargs = super(LVMFormatPopulator, self)._get_kwargs() @@ -247,7 +253,7 @@ def add_lv(lv): lv_parents = [self._devicetree.get_device_by_device_id("LVM-" + pool_device_name)] elif lv_attr[0] == 'd': # vdo pool - if BLOCKDEV_LVM_PLUGIN_VDO.available: + if availability.BLOCKDEV_LVM_PLUGIN_VDO.available: pool_info = blockdev.lvm.vdo_info(vg_name, lv_name) if pool_info: lv_kwargs["compression"] = pool_info.compression diff --git a/blivet/populator/helpers/mdraid.py b/blivet/populator/helpers/mdraid.py index fb1b410ae..e3348c355 100644 --- a/blivet/populator/helpers/mdraid.py +++ b/blivet/populator/helpers/mdraid.py @@ -34,6 +34,7 @@ from ...errors import DeviceError, NoParentsError, MDRaidError from ...flags import flags from ...storage_log import log_method_call +from ...tasks import availability from .devicepopulator import DevicePopulator from .formatpopulator import FormatPopulator @@ -43,6 +44,7 @@ class MDDevicePopulator(DevicePopulator): @classmethod + @availability.blockdev_md_required() def match(cls, data): return (udev.device_is_md(data) and not udev.device_get_md_container(data)) @@ -99,6 +101,11 @@ class MDFormatPopulator(FormatPopulator): priority = 100 _type_specifier = "mdmember" + @classmethod + @availability.blockdev_md_required() + def match(cls, data, device): + return super(cls, cls).match(data, device) + def _get_kwargs(self): kwargs = super(MDFormatPopulator, self)._get_kwargs() kwargs["biosraid"] = udev.device_is_biosraid_member(self.data) diff --git a/blivet/populator/populator.py b/blivet/populator/populator.py index ad7b56fe0..2f664d5cd 100644 --- a/blivet/populator/populator.py +++ b/blivet/populator/populator.py @@ -36,6 +36,7 @@ from ..devices import MDRaidArrayDevice from ..devices import MultipathDevice from ..devices import NoDevice +from ..devices import StorageDevice from ..devicelibs import disk as disklib from ..devicelibs import lvm from .. import formats @@ -292,7 +293,20 @@ def handle_device(self, info, update_orig_fmt=False): helper_class = self._get_device_helper(info) if helper_class is not None: - device = helper_class(self, info).run() + try: + device = helper_class(self, info).run() + except DeviceTreeError as e: + log.error("error handling device %s: %s", name, str(e)) + device = self.get_device_by_name(name, incomplete=True, hidden=True) + if device is None: + try: + parents = self._add_parent_devices(info) + except DeviceTreeError: + parents = [] + + device = StorageDevice(name, parents=parents, uuid=udev.device_get_uuid(info), + exists=True) + self._add_device(device) if not device: log.debug("no device obtained for %s", name) @@ -332,7 +346,10 @@ def handle_format(self, info, device, force=False): helper_class = self._get_format_helper(info, device=device) if helper_class is not None: - helper_class(self, info, device).run() + try: + helper_class(self, info, device).run() + except DeviceTreeError as e: + log.error("error handling formatting on %s: %s", device.name, str(e)) log.info("got format: %s", device.format) diff --git a/blivet/tasks/availability.py b/blivet/tasks/availability.py index d5f3aa850..7e689fc71 100644 --- a/blivet/tasks/availability.py +++ b/blivet/tasks/availability.py @@ -588,6 +588,28 @@ class FSOperation(): BLOCKDEV_SWAP_PLUGIN = blockdev_plugin("libblockdev swap plugin", BLOCKDEV_SWAP_TECH) BLOCKDEV_FS_PLUGIN = blockdev_plugin("libblockdev fs plugin", BLOCKDEV_FS_TECH) + +# libblockdev dependency guards +class BlockDevDependencyGuard(util.DependencyGuard, metaclass=abc.ABCMeta): + def __init__(self, plugin): + self._plugin = plugin + super().__init__() + + def _check_avail(self): + return self._plugin.available + + @property + def error_msg(self): + return "%s is not available" % self._plugin.name + + +blockdev_dm_required = BlockDevDependencyGuard(BLOCKDEV_DM_PLUGIN) +blockdev_crypto_required = BlockDevDependencyGuard(BLOCKDEV_CRYPTO_PLUGIN) +blockdev_loop_required = BlockDevDependencyGuard(BLOCKDEV_LOOP_PLUGIN) +blockdev_lvm_required = BlockDevDependencyGuard(BLOCKDEV_LVM_PLUGIN) +blockdev_md_required = BlockDevDependencyGuard(BLOCKDEV_MDRAID_PLUGIN) +blockdev_mpath_required = BlockDevDependencyGuard(BLOCKDEV_MPATH_PLUGIN) + # applications # fsck DOSFSCK_APP = application("dosfsck") diff --git a/tests/unit_tests/devicetree_test.py b/tests/unit_tests/devicetree_test.py index 1a61a87e4..6ac08ec0c 100644 --- a/tests/unit_tests/devicetree_test.py +++ b/tests/unit_tests/devicetree_test.py @@ -12,6 +12,7 @@ from blivet.devices import StorageDevice from blivet.devices import MultipathDevice from blivet.devices import MDRaidArrayDevice +from blivet.devices import PartitionDevice from blivet.devices.lib import Tags from blivet.devicetree import DeviceTree from blivet.formats import get_format @@ -218,7 +219,13 @@ def test_add_device(self, *args): # pylint: disable=unused-argument # adding a device with the same UUID dev_clone = StorageDevice("dev_clone", exists=False, uuid=sentinel.dev1_uuid, parents=[]) - self.assertRaisesRegex(DuplicateUUIDError, "Duplicate UUID.*", dt._add_device, dev_clone) + with patch("blivet.devicetree.flags") as flags: + flags.allow_inconsistent_config = False + self.assertRaisesRegex(DuplicateUUIDError, "Duplicate UUID.*", dt._add_device, dev_clone) + + flags.allow_inconsistent_config = True + dt._add_device(dev_clone) + dt._remove_device(dev_clone) dev2 = StorageDevice("dev2", exists=False, parents=[]) dev3 = StorageDevice("dev3", exists=False, parents=[dev1, dev2]) @@ -320,6 +327,23 @@ def test_get_device_by_device_id(self): self.assertIsNone(dt.get_device_by_device_id("dev3")) self.assertEqual(dt.get_device_by_device_id("dev3", hidden=True), dev3) + def test_get_device_by_uuid(self): + # basic tests: device uuid, format uuid + dt = DeviceTree() + + dev1 = PartitionDevice('part1', uuid=sentinel.uuid1) + dev2 = PartitionDevice('part2', uuid=sentinel.uuid2) + dt._add_device(dev1) + dt._add_device(dev2) + + self.assertIsNone(dt.get_device_by_uuid(sentinel.uuid3)) + self.assertEqual(dt.get_device_by_uuid(sentinel.uuid1), dev1) + self.assertEqual(dt.get_device_by_uuid(sentinel.uuid2), dev2) + + # multiple matches -> DuplicateUUIDError + dev2.uuid = sentinel.uuid1 + self.assertRaises(DuplicateUUIDError, dt.get_device_by_uuid, sentinel.uuid1) + def test_recursive_remove(self): dt = DeviceTree() dev1 = StorageDevice("dev1", exists=False, parents=[]) diff --git a/tests/unit_tests/populator_test.py b/tests/unit_tests/populator_test.py index 2d8175f2a..c4101eb63 100644 --- a/tests/unit_tests/populator_test.py +++ b/tests/unit_tests/populator_test.py @@ -11,6 +11,7 @@ from blivet.devices import NVMeNamespaceDevice, NVMeFabricsNamespaceDevice from blivet.devicelibs import lvm from blivet.devicetree import DeviceTree +from blivet.errors import DeviceTreeError from blivet.formats import get_device_format_class, get_format, DeviceFormat from blivet.formats.disklabel import DiskLabel from blivet.populator.helpers import DiskDevicePopulator, DMDevicePopulator, LoopDevicePopulator @@ -25,6 +26,70 @@ from blivet.size import Size +class PopulatorTestCase(unittest.TestCase): + @patch.object(DeviceTree, "_get_format_helper") + def test_handle_format_error_handling(self, *args): + """ Test handling of errors raised from populator (format) helpers' run methods. + + The piece we want to test is that DeviceTreeError being raised from the + helper's run method results in no crash and an unset format on the device. + There is no need to test the various helpers individually since the only + machinery is in Populator.handle_format. + """ + get_format_helper_patch = args[0] + + devicetree = DeviceTree() + + # Set up info to look like a specific format type + name = 'fhtest1' + fmt_type = 'xfs' + info = dict(SYS_NAME=name, ID_FS_TYPE=fmt_type) + device = StorageDevice(name, size=Size('50g'), exists=True) + + # Set up helper to raise an exn in run() + helper = Mock() + helper.side_effect = DeviceTreeError("failed to populate format") + get_format_helper_patch.return_value = helper + + devicetree.handle_format(info, device) + + self.assertEqual(device.format.type, None) + + @patch("blivet.static_data.lvm_info.blockdev.lvm.lvs", return_value=[]) + @patch.object(DeviceTree, "_reason_to_skip_device", return_value=None) + @patch.object(DeviceTree, "_clear_new_multipath_member") + @patch.object(DeviceTree, "handle_format") + @patch.object(DeviceTree, "_add_parent_devices") + @patch.object(DeviceTree, "_get_device_helper") + def test_handle_device_error_handling(self, *args): + """ Test handling of errors raised from populator (device) helpers' run methods. + + When the helper's run method raises DeviceTreeError we should end up with a + StorageDevice (and no traceback). There is no need to test the various + helper classes since all of the machinery is in Populator.handle_device. + """ + get_device_helper_patch = args[0] + add_parent_devices_patch = args[1] + + devicetree = DeviceTree() + + # Set up info to look like a specific device type + name = 'dhtest1' + info = dict(SYS_NAME=name, SYS_PATH='/fake/sys/path/dhtest1') + + # Set up helper to raise an exn in run() + helper = Mock() + helper.side_effect = DeviceTreeError("failed to populate device") + get_device_helper_patch.return_value = helper + + add_parent_devices_patch.return_value = list() + devicetree.handle_device(info) + + device = devicetree.get_device_by_name(name) + self.assertIsNotNone(device) + self.assertIsInstance(device, StorageDevice) + + class PopulatorHelperTestCase(unittest.TestCase): helper_class = None