curtin-dev team mailing list archive
-
curtin-dev team
-
Mailing list archive
-
Message #00677
[Merge] ~raharper/curtin:fix/uefi-reorder-missing-boot-current into curtin:master
Ryan Harper has proposed merging ~raharper/curtin:fix/uefi-reorder-missing-boot-current into curtin:master.
Commit message:
UEFI: Handle missing BootCurrent entry when reordering UEFI entries
Curtin typically reorders UEFI boot entries to place the currently
booted entry at the front of the list after install. In some cases a
system UEFI may not present a BootCurrent entry and curtin will not
performance any reordering leaving a node to boot from the newly
installed entry. For MAAS deployments, this causes issues as MAAS
expects to retain control over the node via PXE booting.
Curtin will attempt to reorder the boot menu when BootCurrent is
missing (or when forced via curtin config option) by detecting
which menu item was added by the curtin install and moving it back
once place in the list. The feature, UEFI_REORDER_FALLBACK_SUPPORT
is enabled by default. Users may disable reordering and fallback
support.
- unittest/helpers.py: Add with_logs support for CiTestCase
- docs: document grub: reorder_uefi and grub:
reorder_uefi_force_fallback config options.
LP: #1789650
Requested reviews:
curtin developers (curtin-dev)
Related bugs:
Bug #1789650 in curtin: "Servers set to boot from disk after MAAS installation"
https://bugs.launchpad.net/curtin/+bug/1789650
For more details, see:
https://code.launchpad.net/~raharper/curtin/+git/curtin/+merge/387981
--
Your team curtin developers is requested to review the proposed merge of ~raharper/curtin:fix/uefi-reorder-missing-boot-current into curtin:master.
diff --git a/curtin/__init__.py b/curtin/__init__.py
index ae4ce11..092020b 100644
--- a/curtin/__init__.py
+++ b/curtin/__init__.py
@@ -34,6 +34,8 @@ FEATURES = [
'APT_CONFIG_V1',
# has version module
'HAS_VERSION_MODULE',
+ # uefi_reoder has fallback support if BootCurrent is missing
+ 'UEFI_REORDER_FALLBACK_SUPPORT',
]
__version__ = "20.1"
diff --git a/curtin/commands/curthooks.py b/curtin/commands/curthooks.py
index 458eb9d..5fbd5b8 100644
--- a/curtin/commands/curthooks.py
+++ b/curtin/commands/curthooks.py
@@ -437,18 +437,26 @@ def uefi_remove_old_loaders(grubcfg, target):
"should be removed.", info['name'])
-def uefi_reorder_loaders(grubcfg, target):
+def uefi_reorder_loaders(grubcfg, target, efi_orig=None):
"""Reorders the UEFI BootOrder to place BootCurrent first.
The specifically doesn't try to do to much. The order in which grub places
a new EFI loader is up to grub. This only moves the BootCurrent to the
front of the BootOrder.
+
+ In some systems, BootCurrent may not be set/present. In this case
+ curtin will attempt to place the new boot entry created when grub
+ is installed after the the previous first entry (before we installed grub).
+
"""
if grubcfg.get('reorder_uefi', True):
efi_output = util.get_efibootmgr(target=target)
currently_booted = efi_output.get('current', None)
boot_order = efi_output.get('order', [])
- if currently_booted:
+ new_boot_order = None
+ force_fallback_reorder = config.value_as_boolean(
+ grubcfg.get('reorder_uefi_force_fallback', False))
+ if currently_booted and force_fallback_reorder is False:
if currently_booted in boot_order:
boot_order.remove(currently_booted)
boot_order = [currently_booted] + boot_order
@@ -456,6 +464,40 @@ def uefi_reorder_loaders(grubcfg, target):
LOG.debug(
"Setting currently booted %s as the first "
"UEFI loader.", currently_booted)
+ else:
+ reason = (
+ "config 'reorder_uefi_force_fallback' is True" if
+ force_fallback_reorder else "missing 'BootCurrent' value")
+ LOG.debug("Using fallback UEFI reordering: " + reason)
+ if len(boot_order) < 2:
+ LOG.debug(
+ 'UEFI BootOrder has less than 2 entriesi, cannot reorder')
+ return
+ # look at efi entries before we added one to find the new addition
+ if efi_orig is not None:
+ LOG.debug("Finding newly install entry and moving it back one")
+ # find the newly installed entry, and move it back one
+ # spot, (new entry == 0004)
+ # orig = [0002, 0001, 0003, 0000]
+ # new = [0002, 0004, 0001, 0003, 0000]
+ # \
+ # -> [0002, 0001, 0004, 0003, 0000]
+ new_entries = set(boot_order).difference(
+ set(efi_orig['order']))
+ for entry in new_entries:
+ # insert a copy on the other side of the next
+ new_spot = boot_order.index(entry) + 2
+ boot_order.insert(new_spot, entry)
+ boot_order.remove(entry)
+ else:
+ LOG.debug("Blindly moving first entry back one")
+ # swap the first and second entry in order list
+ # [0000, 0002, 0001] -> [0002, 0000, 0001]
+ boot_order.insert(2, boot_order[0])
+ boot_order = [bootnum for bootnum in boot_order[1:]]
+ new_boot_order = ','.join(boot_order)
+
+ if new_boot_order:
LOG.debug(
"New UEFI boot order: %s", new_boot_order)
with util.ChrootableTarget(target) as in_chroot:
@@ -692,13 +734,14 @@ def setup_grub(cfg, target, osfamily=DISTROS.debian):
update_nvram = grubcfg.get('update_nvram', True)
if uefi_bootable and update_nvram:
+ efi_orig_output = util.get_efibootmgr(target)
uefi_remove_old_loaders(grubcfg, target)
install_grub(instdevs, target, uefi=uefi_bootable, grubcfg=grubcfg)
if uefi_bootable and update_nvram:
+ uefi_reorder_loaders(grubcfg, target, efi_orig_output)
uefi_remove_duplicate_entries(grubcfg, target)
- uefi_reorder_loaders(grubcfg, target)
def update_initramfs(target=None, all_kernels=False):
diff --git a/doc/topics/config.rst b/doc/topics/config.rst
index 7f8396e..5ce1a4b 100644
--- a/doc/topics/config.rst
+++ b/doc/topics/config.rst
@@ -226,6 +226,27 @@ not provided, Curtin will set the value to 'console'. If the ``terminal``
value is 'unmodified' then Curtin will not set any value at all and will
use Grub defaults.
+**reorder_uefi**: *<boolean: default True>*
+
+On UEFI systems curtin will modify the UEFI BootOrder settings to place
+the currently booted entry (BootCurrent) to the first option after installing
+the new target OS into the UEFI boot menu. The result is that the system
+will boot from the same device that it booted to run curtin.
+
+This setting is ignored if *update_nvram* is False.
+
+
+**reorder_uefi_force_fallback**: *<boolean: default False>*
+
+On some UEFI systems the BootCurrent entry may not be present. This can
+cause a system to not boot to the same device that it was previously booting.
+If BootCurrent is not present, curtin will attempt to move the newly added
+boot entry after the previous first entry in BootOrder. If
+*reorder_uefi_force_fallback* is True, curtin will ignore BootCurrent value and
+use the fallback reordering method.
+
+This setting is ignored if *update_nvram* or *reorder_uefi* are False.
+
**Example**::
@@ -264,6 +285,12 @@ use Grub defaults.
probe_additional_os: True
terminal: unmodified
+**Enable Fallback UEFI Reordering**::
+
+ grub:
+ reorder_uefi: true
+ reorder_uefi_force_fallback: true
+
http_proxy
~~~~~~~~~~
diff --git a/tests/unittests/helpers.py b/tests/unittests/helpers.py
index 2f5e51a..64a79ca 100644
--- a/tests/unittests/helpers.py
+++ b/tests/unittests/helpers.py
@@ -2,11 +2,13 @@
import imp
import importlib
+import logging
import mock
import os
import random
import shutil
import string
+import sys
import tempfile
from unittest import TestCase, skipIf
from contextlib import contextmanager
@@ -55,6 +57,7 @@ def skipUnlessJsonSchema():
class CiTestCase(TestCase):
"""Common testing class which all curtin unit tests subclass."""
+ with_logs = False
allowed_subp = False
SUBP_SHELL_TRUE = "shell=True"
@@ -69,6 +72,21 @@ class CiTestCase(TestCase):
def setUp(self):
super(CiTestCase, self).setUp()
+ if self.with_logs:
+ # Create a log handler so unit tests can search expected logs.
+ self.logger = logging.getLogger()
+ if sys.version_info[0] == 2:
+ import StringIO
+ self.logs = StringIO.StringIO()
+ else:
+ import io
+ self.logs = io.StringIO()
+ formatter = logging.Formatter('%(levelname)s: %(message)s')
+ handler = logging.StreamHandler(self.logs)
+ handler.setFormatter(formatter)
+ self.old_handlers = self.logger.handlers
+ self.logger.handlers = [handler]
+
if self.allowed_subp is True:
util.subp = _real_subp
else:
diff --git a/tests/unittests/test_curthooks.py b/tests/unittests/test_curthooks.py
index 2349456..e7b960d 100644
--- a/tests/unittests/test_curthooks.py
+++ b/tests/unittests/test_curthooks.py
@@ -533,6 +533,8 @@ class TestSetupZipl(CiTestCase):
class TestSetupGrub(CiTestCase):
+ with_logs = True
+
def setUp(self):
super(TestSetupGrub, self).setUp()
self.target = self.tmp_dir()
@@ -767,6 +769,68 @@ class TestSetupGrub(CiTestCase):
call(['efibootmgr', '-o', '0001,0000'], target=self.target)],
self.mock_subp.call_args_list)
+ @patch.object(util.ChrootableTarget, "__enter__", new=lambda a: a)
+ def test_grub_install_uefi_updates_nvram_reorders_no_current(self):
+ self.add_patch('curtin.distro.install_packages', 'mock_install')
+ self.add_patch('curtin.distro.has_pkg_available', 'mock_haspkg')
+ self.add_patch('curtin.util.get_efibootmgr', 'mock_efibootmgr')
+ self.mock_is_uefi_bootable.return_value = True
+ cfg = {
+ 'grub': {
+ 'install_devices': ['/dev/vdb'],
+ 'update_nvram': True,
+ 'remove_old_uefi_loaders': False,
+ 'reorder_uefi': True,
+ },
+ }
+ self.mock_efibootmgr.side_effect = iter([
+ {'order': ['0001'], 'entries': {}}, # query1
+ {'order': ['0001'], 'entries': {}}, # install
+ {'order': ['0000', '0001'], 'entries': {}}, # post install
+ {'order': ['0001', '0000'], 'entries': {}}, # remove duplicate
+ ])
+ self.mock_haspkg.return_value = False
+ curthooks.setup_grub(cfg, self.target, osfamily=self.distro_family)
+ self.assertEquals([
+ call(['efibootmgr', '-o', '0001,0000'], target=self.target)],
+ self.mock_subp.call_args_list)
+ self.assertIn("Using fallback UEFI reordering:", self.logs.getvalue())
+ self.assertIn("missing 'BootCurrent' value", self.logs.getvalue())
+
+ @patch.object(util.ChrootableTarget, "__enter__", new=lambda a: a)
+ def test_grub_install_uefi_updates_nvram_reorders_force_fallback(self):
+ self.add_patch('curtin.distro.install_packages', 'mock_install')
+ self.add_patch('curtin.distro.has_pkg_available', 'mock_haspkg')
+ self.add_patch('curtin.util.get_efibootmgr', 'mock_efibootmgr')
+ self.mock_is_uefi_bootable.return_value = True
+ cfg = {
+ 'grub': {
+ 'install_devices': ['/dev/vdb'],
+ 'update_nvram': True,
+ 'remove_old_uefi_loaders': False,
+ 'reorder_uefi': True,
+ 'reorder_uefi_force_fallback': True,
+ },
+ }
+ self.mock_efibootmgr.side_effect = iter([
+ {'current': '0001',
+ 'order': ['0001'], 'entries': {}}, # query1
+ {'current': '0001',
+ 'order': ['0001'], 'entries': {}}, # install
+ {'current': '0001',
+ 'order': ['0000', '0001'], 'entries': {}}, # post install
+ {'current': '0001',
+ 'order': ['0001', '0000'], 'entries': {}}, # remove duplicate
+ ])
+ self.mock_haspkg.return_value = False
+ curthooks.setup_grub(cfg, self.target, osfamily=self.distro_family)
+ self.assertEquals([
+ call(['efibootmgr', '-o', '0001,0000'], target=self.target)],
+ self.mock_subp.call_args_list)
+ self.assertIn("Using fallback UEFI reordering:", self.logs.getvalue())
+ self.assertIn("config 'reorder_uefi_force_fallback' is True",
+ self.logs.getvalue())
+
class TestUefiRemoveDuplicateEntries(CiTestCase):
diff --git a/tests/unittests/test_feature.py b/tests/unittests/test_feature.py
index 84325ef..8690ad8 100644
--- a/tests/unittests/test_feature.py
+++ b/tests/unittests/test_feature.py
@@ -27,4 +27,7 @@ class TestExportsFeatures(CiTestCase):
def test_has_btrfs_swapfile_support(self):
self.assertIn('BTRFS_SWAPFILE', curtin.FEATURES)
+ def test_has_uefi_reorder_fallback_support(self):
+ self.assertIn('UEFI_REORDER_FALLBACK_SUPPORT', curtin.FEATURES)
+
# vi: ts=4 expandtab syntax=python
Follow ups
