diff --git a/blivet/devicetree.py b/blivet/devicetree.py index 25e1816d7..8fb9b1b35 100644 --- a/blivet/devicetree.py +++ b/blivet/devicetree.py @@ -40,6 +40,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 @@ -137,8 +138,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) @@ -159,7 +161,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)) @@ -510,7 +512,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 = six.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 d7d4db994..3c97adf06 100644 --- a/blivet/errors.py +++ b/blivet/errors.py @@ -196,8 +196,7 @@ class DeviceNotFoundError(StorageError): pass -class UnusableConfigurationError(StorageError): - +class UnusableConfigurationError(DeviceTreeError): """ User has an unusable initial storage configuration. """ suggestion = "" diff --git a/blivet/flags.py b/blivet/flags.py index 6500be305..6124b72df 100644 --- a/blivet/flags.py +++ b/blivet/flags.py @@ -81,6 +81,7 @@ def __init__(self): self.update_from_boot_cmdline() self.allow_imperfect_devices = True + self.allow_inconsistent_config = True self.debug_threads = False def get_boot_cmdline(self): diff --git a/blivet/iscsi.py b/blivet/iscsi.py index f612cf15a..69f6420cf 100644 --- a/blivet/iscsi.py +++ b/blivet/iscsi.py @@ -22,7 +22,6 @@ from . import util from .flags import flags from .i18n import _ -from .storage_log import log_exception_info from . import safe_dbus import os import re @@ -160,7 +159,7 @@ def __init__(self): initiatorname = self._call_initiator_method("GetFirmwareInitiatorName")[0] self._initiator = initiatorname except Exception: # pylint: disable=broad-except - log_exception_info(fmt_str="failed to get initiator name from iscsi firmware") + log.info("No iSCSI firmware initiator found") # So that users can write iscsi() to get the singleton instance def __call__(self): @@ -278,7 +277,7 @@ def _get_active_sessions(self): 'GetManagedObjects', None)[0] except safe_dbus.DBusCallError: - log_exception_info(log.info, "iscsi: Failed to get active sessions.") + log.info("iscsi: Failed to get active sessions.") return [] sessions = (obj for obj in objects.keys() if re.match(r'.*/iscsi/session[0-9]+$', obj)) @@ -303,7 +302,7 @@ def _start_ibft(self): try: found_nodes, _n_nodes = self._call_initiator_method("DiscoverFirmware", args) except safe_dbus.DBusCallError: - log_exception_info(log.info, "iscsi: No IBFT info found.") + log.info("iscsi: No IBFT info found.") # an exception here means there is no ibft firmware, just return return diff --git a/blivet/populator/populator.py b/blivet/populator/populator.py index 465c272db..f584b93bf 100644 --- a/blivet/populator/populator.py +++ b/blivet/populator/populator.py @@ -37,6 +37,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 @@ -262,7 +263,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_slave_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) @@ -302,7 +316,10 @@ def handle_format(self, info, device): 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/udev.py b/blivet/udev.py index c53bbdf1a..dbf67904f 100644 --- a/blivet/udev.py +++ b/blivet/udev.py @@ -202,7 +202,7 @@ def device_get_name(udev_info): """ Return the best name for a device based on the udev db data. """ if "DM_NAME" in udev_info: name = udev_info["DM_NAME"] - elif "MD_DEVNAME" in udev_info: + elif "MD_DEVNAME" in udev_info and os.path.exists(device_get_sysfs_path(udev_info) + "/md"): mdname = udev_info["MD_DEVNAME"] if device_is_partition(udev_info): # for partitions on named RAID we want to use the raid name, not diff --git a/tests/devicetree_test.py b/tests/devicetree_test.py index 181bad4cb..d30f05a54 100644 --- a/tests/devicetree_test.py +++ b/tests/devicetree_test.py @@ -11,6 +11,7 @@ from blivet.devices import LVMVolumeGroupDevice from blivet.devices import StorageDevice from blivet.devices import MultipathDevice +from blivet.devices import PartitionDevice from blivet.devices.lib import Tags from blivet.devicetree import DeviceTree from blivet.formats import get_format @@ -121,7 +122,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=[]) - six.assertRaisesRegex(self, DuplicateUUIDError, "Duplicate UUID.*", dt._add_device, dev_clone) + with patch("blivet.devicetree.flags") as flags: + flags.allow_inconsistent_config = False + six.assertRaisesRegex(self, 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]) @@ -199,6 +206,23 @@ def test_get_device_by_name(self): self.assertIsNone(dt.get_device_by_name("dev3")) self.assertEqual(dt.get_device_by_name("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/populator_test.py b/tests/populator_test.py index 84e79ff37..5cd208ee3 100644 --- a/tests/populator_test.py +++ b/tests/populator_test.py @@ -12,6 +12,7 @@ from blivet.devices import MDRaidArrayDevice, MultipathDevice, OpticalDevice from blivet.devices import PartitionDevice, StorageDevice, NVDIMMNamespaceDevice 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,69 @@ 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.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_slave_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_slave_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_slave_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