curtin-dev team mailing list archive

[Michael Hudson-Doyle <mp+407568@xxxxxxxxxxxxxxxxxx>]

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

Commit message:
block:lvm: search encrypted volumes for LVM

This reverts my optimistic fix for bug 1895192 and replaces it with Lukas' more cautious fix.

We should debug why things are failing with my fix but not with 20.04.3 imminent.

LP: #1940687

Requested reviews:
  curtin developers (curtin-dev)
Related bugs:
  Bug #1895192 in curtin: "Optimistic approach of using zkey for encrypted installations on s390x not working"
  https://bugs.launchpad.net/curtin/+bug/1895192

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

diff --git a/curtin/block/lvm.py b/curtin/block/lvm.py
index bd0f1aa..333b46f 100644
--- a/curtin/block/lvm.py
+++ b/curtin/block/lvm.py
@@ -81,7 +81,7 @@ def activate_volgroups(multipath=False):
     """
     cmd = ['vgchange', '--activate=y']
     if multipath:
-        # only operate on mp devices
+        # only operate on mp devices or encrypted volumes
         mp_filter = generate_multipath_dev_mapper_filter()
         cmd.extend(['--config', 'devices{ %s }' % mp_filter])
 
@@ -100,12 +100,14 @@ def _generate_multipath_filter(accept=None):
 
 
 def generate_multipath_dev_mapper_filter():
-    return _generate_multipath_filter(accept=['/dev/mapper/mpath.*'])
+    return _generate_multipath_filter(
+        accept=['/dev/mapper/mpath.*', '/dev/mapper/dm_crypt-.*'])
 
 
 def generate_multipath_dm_uuid_filter():
-    return _generate_multipath_filter(
-        accept=['/dev/disk/by-id/dm-uuid-.*mpath-.*'])
+    return _generate_multipath_filter(accept=[
+        '/dev/disk/by-id/dm-uuid-.*mpath-.*',
+        '/dev/disk/by-id/.*dm_crypt-.*'])
 
 
 def lvm_scan(activate=True, multipath=False):
@@ -126,8 +128,9 @@ def lvm_scan(activate=True, multipath=False):
         release = 'xenial'
 
     if multipath:
-        # only operate on mp devices
-        mponly = 'devices{ filter = [ "a|/dev/mapper/mpath.*|", "r|.*|" ] }'
+        # only operate on mp devices or encrypted volumes
+        mponly = 'devices{ filter = [ "a|%s|", "a|%s|", "r|.*|" ] }' % (
+            '/dev/mapper/mpath.*', '/dev/mapper/dm_crypt-.*')
 
     for cmd in [['pvscan'], ['vgscan']]:
         if release != 'precise' and lvmetad_running():
diff --git a/curtin/commands/curthooks.py b/curtin/commands/curthooks.py
index f78bd21..8ed4f5f 100644
--- a/curtin/commands/curthooks.py
+++ b/curtin/commands/curthooks.py
@@ -13,6 +13,7 @@ from curtin import config
 from curtin import block
 from curtin import distro
 from curtin.block import iscsi
+from curtin.block import lvm
 from curtin import net
 from curtin import futil
 from curtin.log import LOG
@@ -1143,7 +1144,28 @@ def detect_and_handle_multipath(cfg, target, osfamily=DISTROS.debian):
             raise ValueError(
                     'Unknown grub_cfg mapping for distro: %s' % osfamily)
 
-        if not mp_supported:
+        if mp_supported:
+            # if root is on lvm, emit a multipath filter to lvm
+            lvmfilter = lvm.generate_multipath_dm_uuid_filter()
+            # lvm.conf device section indents config by 8 spaces
+            indent = ' ' * 8
+            mpfilter = '\n'.join([
+                indent + ('# Modified by curtin for multipath '
+                          'device %s' % (mpname)),
+                indent + lvmfilter])
+            lvmconf = paths.target_path(target, '/etc/lvm/lvm.conf')
+            orig_content = util.load_file(lvmconf)
+            devices_match = re.search(r'devices\ {',
+                                      orig_content, re.MULTILINE)
+            if devices_match:
+                LOG.debug('Adding multipath filter (%s) to lvm.conf', mpfilter)
+                shutil.move(lvmconf, lvmconf + '.orig-curtin')
+                index = devices_match.end()
+                new_content = (
+                    orig_content[:index] + '\n' + mpfilter + '\n' +
+                    orig_content[index + 1:])
+                util.write_file(lvmconf, new_content)
+        else:
             # TODO: fix up dnames without multipath available on host
             msg = '\n'.join([
                 '# Written by curtin for multipath device %s %s' % (mpname,
diff --git a/tests/unittests/test_block_lvm.py b/tests/unittests/test_block_lvm.py
index ff58b30..fc6130a 100644
--- a/tests/unittests/test_block_lvm.py
+++ b/tests/unittests/test_block_lvm.py
@@ -106,7 +106,8 @@ class TestBlockLvm(CiTestCase):
         lvm.lvm_scan(multipath=True)
         cmd_filter = [
             '--config',
-            'devices{ filter = [ "a|/dev/mapper/mpath.*|", "r|.*|" ] }'
+            'devices{ filter = [ "a|%s|", "a|%s|", "r|.*|" ] }' % (
+                '/dev/mapper/mpath.*', '/dev/mapper/dm_crypt-.*')
         ]
         expected = [cmd + cmd_filter for cmd in cmds]
         calls = [mock.call(cmd, capture=True) for cmd in expected]
@@ -117,12 +118,15 @@ class TestBlockLvm(CiTestCase):
 class TestBlockLvmMultipathFilter(CiTestCase):
 
     def test_generate_multipath_dev_mapper_filter(self):
-        expected = 'filter = [ "a|/dev/mapper/mpath.*|", "r|.*|" ]'
+        expected = 'filter = [ "a|%s|", "a|%s|", "r|.*|" ]' % (
+            '/dev/mapper/mpath.*', '/dev/mapper/dm_crypt-.*')
         self.assertEqual(expected, lvm.generate_multipath_dev_mapper_filter())
 
     def test_generate_multipath_dm_uuid_filter(self):
         expected = (
-            'filter = [ "a|/dev/disk/by-id/dm-uuid-.*mpath-.*|", "r|.*|" ]')
+            'filter = [ "a|%s|", "a|%s|", "r|.*|" ]' % (
+                '/dev/disk/by-id/dm-uuid-.*mpath-.*',
+                '/dev/disk/by-id/.*dm_crypt-.*'))
         self.assertEqual(expected, lvm.generate_multipath_dm_uuid_filter())