curtin-dev team mailing list archive

[Michael Hudson-Doyle <mwhudsonlp@xxxxxxxxxxx>]

Michael Hudson-Doyle has proposed merging ~mwhudson/curtin:lp-1878041 into curtin:master.

Commit message:
multipath: make find_mpath_id much cheaper

This function asked multipathd to list all maps and read out the name
(or wwid if no name) of the device from that, but this information is
already present in udev as DM_NAME -- and DM_NAME is the wwid if
user_friendly_names is disabled, so there's no need to be conditional
about that either.

LP: #1878041


Requested reviews:
  curtin developers (curtin-dev)
Related bugs:
  Bug #1878041 in curtin: "installation fails creating user on large multipath system - multipathd show command times out in curtin log with 163 paths"
  https://bugs.launchpad.net/curtin/+bug/1878041

For more details, see:
https://code.launchpad.net/~mwhudson/curtin/+git/curtin/+merge/396538
-- 
Your team curtin developers is requested to review the proposed merge of ~mwhudson/curtin:lp-1878041 into curtin:master.

diff --git a/curtin/block/multipath.py b/curtin/block/multipath.py
index 7ad1791..01bc80e 100644
--- a/curtin/block/multipath.py
+++ b/curtin/block/multipath.py
@@ -139,19 +139,10 @@ def find_mpath_members(multipath_id, paths=None):
     return members
 
 
-def find_mpath_id(devpath, maps=None):
+def find_mpath_id(devpath):
     """ Return the mpath_id associated with a specified device path. """
-    if not maps:
-        maps = show_maps()
-
-    for mpmap in maps:
-        if '/dev/' + mpmap['sysfs'] == devpath:
-            name = mpmap.get('name')
-            if name:
-                return name
-            return mpmap['multipath']
-
-    return None
+    info = udev.udevadm_info(devpath)
+    return info.get('DM_NAME')
 
 
 def find_mpath_id_by_path(devpath, paths=None):
diff --git a/tests/unittests/test_block_multipath.py b/tests/unittests/test_block_multipath.py
index 96cbcba..1827f4a 100644
--- a/tests/unittests/test_block_multipath.py
+++ b/tests/unittests/test_block_multipath.py
@@ -170,24 +170,14 @@ class TestMultipath(CiTestCase):
 
     def test_find_mpath_id(self):
         """find_mpath_id returns mpath_id if device is part of mpath group."""
-        mp_id = 'mpatha'
-        maps = ['sysfs=bar multipath=mpatha',
-                'sysfs=wark multipath=mpatha']
-        self.m_subp.return_value = ("\n".join(maps), "")
-        self.assertEqual(mp_id, multipath.find_mpath_id('/dev/bar'))
-
-    def test_find_mpath_id_name(self):
-        """find_mpath_id_name returns the name if present in maps on match."""
-        maps = ['sysfs=bar multipath=mpatha name=friendly',
-                'sysfs=wark multipath=mpatha']
-        self.m_subp.return_value = ("\n".join(maps), "")
-        self.assertEqual('friendly', multipath.find_mpath_id('/dev/bar'))
+        self.m_udev.udevadm_info.return_value = {
+            'DM_NAME': 'mpatha-foo'
+            }
+        self.assertEqual('mpatha-foo', multipath.find_mpath_id('/dev/bar'))
 
     def test_find_mpath_id_none(self):
         """find_mpath_id_name returns none when device is not part of maps."""
-        maps = ['sysfs=bar multipath=mpatha',
-                'sysfs=wark multipath=mpatha']
-        self.m_subp.return_value = ("\n".join(maps), "")
+        self.m_udev.udevadm_info.return_value = {}
         self.assertEqual(None, multipath.find_mpath_id('/dev/foo'))
 
     def test_dmname_to_blkdev_mapping(self):