RFC: Avoid some tracebacks during populate

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
 

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 = ""
 

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):

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
 

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)
 

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

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=[])

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