Enable separate /boot and /boot/esp via XBOOTLDR in systemd-boot (#1859)

* Disabled /boot check for now

* Making '/boot' more dynamic

* str() on boot_partition didn't work

* _pacstrap -> pacman.strap()

* Added 'finding' the EFI partition logic

* f-string qotations

* Locked down so get_boot_partition() looks for /boot and get_efi_partition() looks for /boot/efi - essentially hardcoding it for now, as there's no easy way to distinguish between the EFI partition or BOOT partition if they are both FAT32 for some reason.

* Added some debugging output

* Fixed some mypy complaints

* Fixed PosixPath() vs str comparison

* Changed FAT32 comparitor, should be FilesystemType.Fat32 now

* Fixed PosixPath() vs str comparison

* Re-ordered _add_systemd_bootloader() argument order, to match the other functions. This will cause the function to break on scripts that call this explicitly.

* is_boot() now returns True if any type of valid boot flags are set, not just the 'Boot' flag. This allows us to check for XBOOTLDR flag as well.

* Converted static INT to _ped.PARTITION_<flag> definition. This matches the way pyparted checks for flags on partitions.

* /boot/efi -> /boot/EFI  (while the recommendation from bootctl is to mount it to /efi, I want to test it with custom paths first)

* Removed _ped from mypy checks

* flake8 fix

* Added ESP flag to partitions

* Added more docs in the docstring

* Renamed *efi_partition to *xbootldr_partition within this PR changes

* Naming collision, PartitionType -> PartitionGUIDs to avoid overwriting existing PartitionType

* Check for XBOOTLDR instead of fixed EFI mountpoint in get_xbootldr_partition()

* Mixed up XBOOTLDR and EFI partitions a bit, brought back get_efi_partition() which now filters out XBOOTLDR partitions and only returns a partition when there is a boot partition found by get_boot_partition()

* Fixed symbiosis between get_boot() and get_efi() so that they don't report the same potential partition

* Removed debugging code

* Improved comments surrounding why /loader/ rather than /loader/ - this may change
This commit is contained in:
Anton Hvornum 2023-07-17 00:14:44 +02:00 committed by GitHub
parent 21735c57ca
commit afaf42e646
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 73 additions and 15 deletions

View File

@ -13,6 +13,7 @@ from typing import Optional, List, Dict, TYPE_CHECKING, Any
from typing import Union
import parted # type: ignore
import _ped # type: ignore
from parted import Disk, Geometry, Partition
from ..exceptions import DiskError, SysCallError
@ -525,7 +526,21 @@ class PartitionType(Enum):
class PartitionFlag(Enum):
Boot = 1
"""
Flags are taken from _ped because pyparted uses this to look
up their flag definitions: https://github.com/dcantrell/pyparted/blob/c4e0186dad45c8efbe67c52b02c8c4319df8aa9b/src/parted/__init__.py#L200-L202
Which is the way libparted checks for its flags: https://git.savannah.gnu.org/gitweb/?p=parted.git;a=blob;f=libparted/labels/gpt.c;hb=4a0e468ed63fff85a1f9b923189f20945b32f4f1#l183
"""
Boot = _ped.PARTITION_BOOT
XBOOTLDR = _ped.PARTITION_BLS_BOOT # Note: parted calls this bls_boot
ESP = _ped.PARTITION_ESP
# class PartitionGUIDs(Enum):
# """
# A list of Partition type GUIDs (lsblk -o+PARTTYPE) can be found here: https://en.wikipedia.org/wiki/GUID_Partition_Table#Partition_type_GUIDs
# """
# XBOOTLDR = 'bc13c2ff-59e6-4262-a352-b275fd6f7172'
class FilesystemType(Enum):
@ -605,6 +620,8 @@ class PartitionModification:
partuuid: Optional[str] = None
uuid: Optional[str] = None
_boot_indicator_flags = [PartitionFlag.Boot, PartitionFlag.XBOOTLDR]
def __post_init__(self):
# needed to use the object as a dictionary key due to hash func
if not hasattr(self, '_obj_id'):
@ -674,7 +691,10 @@ class PartitionModification:
raise ValueError('Mountpoint is not specified')
def is_boot(self) -> bool:
return PartitionFlag.Boot in self.flags
"""
Returns True if any of the boot indicator flags are found in self.flags
"""
return any(set(self.flags) & set(self._boot_indicator_flags))
def is_root(self, relative_mountpoint: Optional[Path] = None) -> bool:
if relative_mountpoint is not None and self.mountpoint is not None:
@ -765,9 +785,24 @@ class DeviceModification:
def add_partition(self, partition: PartitionModification):
self.partitions.append(partition)
def get_efi_partition(self) -> Optional[PartitionModification]:
"""
Similar to get_boot_partition() but excludes XBOOTLDR partitions from it's candidates.
"""
fliltered = filter(lambda x: x.is_boot() and x.fs_type == FilesystemType.Fat32 and PartitionFlag.XBOOTLDR not in x.flags, self.partitions)
return next(fliltered, None)
def get_boot_partition(self) -> Optional[PartitionModification]:
liltered = filter(lambda x: x.is_boot(), self.partitions)
return next(liltered, None)
"""
Returns the first partition marked as XBOOTLDR (PARTTYPE id of bc13c2ff-...) or Boot and has a mountpoint.
Only returns XBOOTLDR if separate EFI is detected using self.get_efi_partition()
"""
if efi_partition := self.get_efi_partition():
fliltered = filter(lambda x: x.is_boot() and x != efi_partition and x.mountpoint, self.partitions)
else:
fliltered = filter(lambda x: x.is_boot() and x.mountpoint, self.partitions)
return next(fliltered, None)
def get_root_partition(self, relative_path: Optional[Path]) -> Optional[PartitionModification]:
filtered = filter(lambda x: x.is_root(relative_path), self.partitions)
@ -886,6 +921,7 @@ class LsblkInfo:
rota: bool = False
tran: Optional[str] = None
partuuid: Optional[str] = None
parttype :Optional[str] = None
uuid: Optional[str] = None
fstype: Optional[str] = None
fsver: Optional[str] = None
@ -909,6 +945,7 @@ class LsblkInfo:
'rota': self.rota,
'tran': self.tran,
'partuuid': self.partuuid,
'parttype' : self.parttype,
'uuid': self.uuid,
'fstype': self.fstype,
'fsver': self.fsver,

View File

@ -172,7 +172,7 @@ class Installer:
)
def sanity_check(self):
self._verify_boot_part()
# self._verify_boot_part()
self._verify_service_stop()
def mount_ordered_layout(self):
@ -677,6 +677,12 @@ class Installer:
else:
raise ValueError(f"Archinstall currently only supports setting up swap on zram")
def _get_efi_partition(self) -> Optional[disk.PartitionModification]:
for layout in self._disk_config.device_modifications:
if partition := layout.get_efi_partition():
return partition
return None
def _get_boot_partition(self) -> Optional[disk.PartitionModification]:
for layout in self._disk_config.device_modifications:
if boot := layout.get_boot_partition():
@ -689,7 +695,12 @@ class Installer:
return root
return None
def _add_systemd_bootloader(self, root_partition: disk.PartitionModification):
def _add_systemd_bootloader(
self,
boot_partition: disk.PartitionModification,
root_partition: disk.PartitionModification,
efi_partition: Optional[disk.PartitionModification]
):
self.pacman.strap('efibootmgr')
if not SysInfo.has_uefi():
@ -698,15 +709,24 @@ class Installer:
# TODO: Ideally we would want to check if another config
# points towards the same disk and/or partition.
# And in which case we should do some clean up.
bootctl_options = [
f'--esp-path={efi_partition.mountpoint}' if efi_partition else '',
f'--boot-path={boot_partition.mountpoint}' if boot_partition else ''
]
# Install the boot loader
try:
SysCommand(f'/usr/bin/arch-chroot {self.target} bootctl --esp-path=/boot install')
SysCommand(f"/usr/bin/arch-chroot {self.target} bootctl {' '.join(bootctl_options)} install")
except SysCallError:
# Fallback, try creating the boot loader without touching the EFI variables
SysCommand(f'/usr/bin/arch-chroot {self.target} bootctl --no-variables --esp-path=/boot install')
SysCommand(f"/usr/bin/arch-chroot {self.target} bootctl --no-variables {' '.join(bootctl_options)} install")
# Ensure that the /boot/loader directory exists before we try to create files in it
# Ensure that the $BOOT/loader/ directory exists before we try to create files in it.
#
# As mentioned in https://github.com/archlinux/archinstall/pull/1859 - we store the
# loader entries in $BOOT/loader/ rather than $ESP/loader/
# The current reasoning being that $BOOT works in both use cases as well
# as being tied to the current installation. This may change.
loader_dir = self.target / 'boot/loader'
loader_dir.mkdir(parents=True, exist_ok=True)
@ -732,7 +752,7 @@ class Installer:
else:
loader.write(f"{line}\n")
# Ensure that the /boot/loader/entries directory exists before we try to create files in it
# Ensure that the $BOOT/loader/entries/ directory exists before we try to create files in it
entries_dir = loader_dir / 'entries'
entries_dir.mkdir(parents=True, exist_ok=True)
@ -836,12 +856,12 @@ class Installer:
self.pacman.strap('efibootmgr') # TODO: Do we need? Yes, but remove from minimal_installation() instead?
try:
SysCommand(f'/usr/bin/arch-chroot {self.target} grub-install --debug --target=x86_64-efi --efi-directory=/boot --bootloader-id=GRUB --removable', peek_output=True)
SysCommand(f'/usr/bin/arch-chroot {self.target} grub-install --debug --target=x86_64-efi --efi-directory={boot_partition.mountpoint} --bootloader-id=GRUB --removable', peek_output=True)
except SysCallError:
try:
SysCommand(f'/usr/bin/arch-chroot {self.target} grub-install --debug --target=x86_64-efi --efi-directory=/boot --bootloader-id=GRUB --removable', peek_output=True)
SysCommand(f'/usr/bin/arch-chroot {self.target} grub-install --debug --target=x86_64-efi --efi-directory={boot_partition.mountpoint} --bootloader-id=GRUB --removable', peek_output=True)
except SysCallError as err:
raise DiskError(f"Could not install GRUB to {self.target}/boot: {err}")
raise DiskError(f"Could not install GRUB to {self.target}{boot_partition.mountpoint}: {err}")
else:
device = disk.device_handler.get_device_by_partition_path(boot_partition.safe_dev_path)
@ -861,7 +881,7 @@ class Installer:
raise DiskError(f"Failed to install GRUB boot on {boot_partition.dev_path}: {err}")
try:
SysCommand(f'/usr/bin/arch-chroot {self.target} grub-mkconfig -o /boot/grub/grub.cfg')
SysCommand(f'/usr/bin/arch-chroot {self.target} grub-mkconfig -o {boot_partition.mountpoint}/grub/grub.cfg')
except SysCallError as err:
raise DiskError(f"Could not configure GRUB: {err}")
@ -1055,6 +1075,7 @@ TIMEOUT=5
if plugin.on_add_bootloader(self):
return True
efi_partition = self._get_efi_partition()
boot_partition = self._get_boot_partition()
root_partition = self._get_root_partition()
@ -1068,7 +1089,7 @@ TIMEOUT=5
match bootloader:
case Bootloader.Systemd:
self._add_systemd_bootloader(root_partition)
self._add_systemd_bootloader(boot_partition, root_partition, efi_partition)
case Bootloader.Grub:
self._add_grub_bootloader(boot_partition, root_partition)
case Bootloader.Efistub: