curtin-dev team mailing list archive

[Ryan Harper <ryan.harper@xxxxxxxxxxxxx>]

Ryan Harper has proposed merging ~raharper/curtin:fix/force-mpath-mapper-links into curtin:master.

Commit message:
multipath: attempt to enforce /dev/mapper/mpath files are symlinks

Multipath device scanning and bring up variablity may result in
the creation if /dev/mapper/mpath* files that are block devices
instead of a symlink to the device mapper device (/dev/dm-0).

When these block files are present, this breaks assumptions in
curtin used to look up details in sysfs for a mapper device.

Requested reviews:
  curtin developers (curtin-dev)

For more details, see:
https://code.launchpad.net/~raharper/curtin/+git/curtin/+merge/382798
-- 
Your team curtin developers is requested to review the proposed merge of ~raharper/curtin:fix/force-mpath-mapper-links into curtin:master.

diff --git a/curtin/block/clear_holders.py b/curtin/block/clear_holders.py
index 279fad8..116ee81 100644
--- a/curtin/block/clear_holders.py
+++ b/curtin/block/clear_holders.py
@@ -684,6 +684,7 @@ def start_clear_holders_deps():
     if mp_support:
         LOG.debug('Detected multipath support, reload maps')
         multipath.reload()
+        multipath.force_devmapper_symlinks()
 
     # scan and activate for logical volumes
     lvm.lvm_scan(multipath=mp_support)
diff --git a/curtin/block/multipath.py b/curtin/block/multipath.py
index 21fc0bd..ead5063 100644
--- a/curtin/block/multipath.py
+++ b/curtin/block/multipath.py
@@ -208,6 +208,31 @@ def get_mpath_id_from_device(device):
     return None
 
 
+def force_devmapper_symlinks():
+    """Check if /dev/mapper/mpath* files are symlinks, if not trigger udev."""
+    LOG.debug('Verifying /dev/mapper/mpath* files are symlinks')
+    needs_trigger = []
+    for mp_id, dm_dev in dmname_to_blkdev_mapping().items():
+        if mp_id.startswith('mpath'):
+            mapper_path = '/dev/mapper/' + mp_id
+            if not os.path.islink(mapper_path):
+                LOG.warning(
+                    'Found invalid device mapper mp path: %s, removing',
+                    mapper_path)
+                util.del_file(mapper_path)
+                needs_trigger.append((mapper_path, dm_dev))
+
+    if len(needs_trigger):
+        for (mapper_path, dm_dev) in needs_trigger:
+            LOG.debug('multipath: regenerating symlink for %s (%s)',
+                      mapper_path, dm_dev)
+            util.subp(['udevadm', 'test', '--action=add',
+                       '/sys/class/block/' + os.path.basename(dm_dev)])
+            udev.udevadm_settle(exists=mapper_path)
+            if not os.path.islink(mapper_path):
+                LOG.error('Failed to regenerate udev symlink %s', mapper_path)
+
+
 def reload():
     """ Request multipath to force reload devmaps. """
     util.subp(['multipath', '-r'])
diff --git a/tests/unittests/test_block_multipath.py b/tests/unittests/test_block_multipath.py
index a0783db..4b8422d 100644
--- a/tests/unittests/test_block_multipath.py
+++ b/tests/unittests/test_block_multipath.py
@@ -227,5 +227,48 @@ class TestMultipath(CiTestCase):
         self.m_subp.return_value = ("\n".join(paths), "")
         self.assertIsNone(multipath.find_mpath_id_by_path('/dev/xxx'))
 
+    @mock.patch('curtin.block.multipath.util.del_file')
+    @mock.patch('curtin.block.multipath.os.path.islink')
+    @mock.patch('curtin.block.multipath.dmname_to_blkdev_mapping')
+    def test_force_devmapper_symlinks(self, m_blkmap, m_islink, m_del_file):
+        """ensure non-symlink for /dev/mapper/mpath* files are regenerated."""
+        m_blkmap.return_value = {
+            'mpatha': '/dev/dm-0',
+            'mpatha-part1': '/dev/dm-1',
+            '1gb zero': '/dev/dm-2',
+        }
+
+        m_islink.side_effect = iter([
+            False, False,  # mpatha, mpath-part1 are not links
+            True, True,    # mpatha, mpath-part1 are symlinks
+        ])
+
+        multipath.force_devmapper_symlinks()
+
+        udev = ['udevadm', 'test', '--action=add']
+        subp_expected_calls = [
+            mock.call(udev + ['/sys/class/block/dm-0']),
+            mock.call(udev + ['/sys/class/block/dm-1']),
+        ]
+        # sorted for py27, whee!
+        self.assertEqual(sorted(subp_expected_calls),
+                         sorted(self.m_subp.call_args_list))
+
+        islink_expected_calls = [
+            mock.call('/dev/mapper/mpatha'),
+            mock.call('/dev/mapper/mpatha-part1'),
+            mock.call('/dev/mapper/mpatha'),
+            mock.call('/dev/mapper/mpatha-part1'),
+        ]
+        self.assertEqual(sorted(islink_expected_calls),
+                         sorted(m_islink.call_args_list))
+
+        del_file_expected_calls = [
+            mock.call('/dev/mapper/mpatha'),
+            mock.call('/dev/mapper/mpatha-part1'),
+        ]
+        self.assertEqual(sorted(del_file_expected_calls),
+                         sorted(m_del_file.call_args_list))
+
 
 # vi: ts=4 expandtab syntax=python