Merge pull request #1306 from vojtechtrefny/main_populate-safe

Avoid some tracebacks during populate

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
 

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

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

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

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

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)
 

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

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)

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)
 

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

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

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