From aa3b79cdd55e0f840ac0095bd84f47ae466cfe5f Mon Sep 17 00:00:00 2001 From: Anton Hvornum Date: Wed, 26 Jan 2022 23:35:23 +0100 Subject: [PATCH] Fixing 781 and it's btrfs mountpoints and kernel parameters (#905) * Added missing import * Brought back the mounting of subvolumes, or at least the beginning of it. Improved some logging and decreased the usage of try/except where unessecary (we can catch it higher up without catching and raising the same thing. * Trying to flatten the mount information to allow for sorted subvolume mounting. Still got issues. * Re-worked mountpoints logic to be a flat dictionary structure, where all subvolumes are mountpoint targets, but with relevant information. Had to work that into Partition().mount to support re-mounting a partition multiple times. That will have to be cleaned up. * Changed the default timeout to 15 as per the ISO * Corrected a path when creating loopdev name. * Added some debugging/warning * Fixed a path lookup issue * Added some debugging * Added more debugging * Removed excessive luks add_key() and crypttab() entries being generated by subvolumes. * Added more debugging * Avoiding excessive keyfile usage for partitions. Also added some debugging for BlockDevice. * Added more debugging * Added debugging and a hack to find /dev/sda from /dev/sda2 * Fixed ugly hack to use /dev/sdX instead of just sdX * Inconsistency in lsblk -no pkname made sorting a bit tricky, resorting to shortest string wins. * Tweak lookup * Tweak lookup * Added more debugging * Had to filter lsblk output because some entries are empty rows * Syntax issue * Added a comment clarifying the many steps of partition->blockdevice logic. * Added debug information * Added debug information * Added debug information * Fixed issue #908 * Added subvolume support for grub. Also added check to see if grub is installed before installing it (to avoid multiple calls to pacstrap even tho it doesn't hurt) * Modified 'sed' to use % as divider, since the replacement string can contain '/' * Added debugging * Testing GRUB with /boot/efi instead (might revert) * Reverted /boot/efi for GRUB. Did no difference. * Flake8 fix * Testing --removable from #793 if it solves the UEFI issue * Removing --remoable and pulling it from #793 for tracability * Update installer.py (#1) (#793) Added "--removable" after "--bootloader-id=GRUB" on Line 669, because it would throw an input/output error without it on my laptop * Remove debug information Co-authored-by: OneLongneck <95297344+OneLongneck@users.noreply.github.com> --- archinstall/lib/disk/blockdevice.py | 11 +- archinstall/lib/disk/btrfs.py | 178 +++++++++++++++------------- archinstall/lib/disk/filesystem.py | 5 +- archinstall/lib/disk/helpers.py | 23 +++- archinstall/lib/disk/partition.py | 52 ++++---- archinstall/lib/disk/user_guides.py | 4 +- archinstall/lib/installer.py | 90 +++++++++----- archinstall/lib/systemd.py | 5 +- archinstall/lib/user_interaction.py | 3 +- 9 files changed, 221 insertions(+), 150 deletions(-) diff --git a/archinstall/lib/disk/blockdevice.py b/archinstall/lib/disk/blockdevice.py index 96caf0fc28..65f4e77d3a 100644 --- a/archinstall/lib/disk/blockdevice.py +++ b/archinstall/lib/disk/blockdevice.py @@ -13,7 +13,10 @@ def __init__(self, path, info=None): from .helpers import all_disks # If we don't give any information, we need to auto-fill it. # Otherwise any subsequent usage will break. - info = all_disks()[path].info + try: + info = all_disks()[path].info + except KeyError: + raise KeyError(f"Could not find {path} in {all_disks()} for some reason.") self.path = path self.info = info @@ -241,7 +244,7 @@ def get_partition(self, uuid): count += 1 else: log(f"Could not find {uuid} in disk after 5 retries",level=logging.INFO) - print(f"Cache: {self.part_cache}") - print(f"Partitions: {self.partitions.items()}") - print(f"UUID: {[uuid]}") + log(f"Cache: {self.part_cache}", level=logging.DEBUG) + log(f"Partitions: {self.partitions.items()}", level=logging.DEBUG) + log(f"UUID: {[uuid]}", level=logging.DEBUG) raise DiskError(f"New partition {uuid} never showed up after adding new partition on {self}") diff --git a/archinstall/lib/disk/btrfs.py b/archinstall/lib/disk/btrfs.py index fb9712f8ce..175d38af8d 100644 --- a/archinstall/lib/disk/btrfs.py +++ b/archinstall/lib/disk/btrfs.py @@ -92,87 +92,99 @@ def manage_btrfs_subvolumes(installation, partition :dict, mountpoints :dict, su # th mount info dict has an entry for the path of the mountpoint (named 'mountpoint') and 'options' which is a list # of mount options (or similar used by brtfs) for name, right_hand in subvolumes.items(): - try: - # we normalize the subvolume name (getting rid of slash at the start if exists. In our implemenation has no semantic load - every subvolume is created from the top of the hierarchy- and simplifies its further use - if name.startswith('/'): - name = name[1:] - # renormalize the right hand. + # we normalize the subvolume name (getting rid of slash at the start if exists. In our implemenation has no semantic load - every subvolume is created from the top of the hierarchy- and simplifies its further use + if name.startswith('/'): + name = name[1:] + + # renormalize the right hand. + location = None + mount_options = [] + # no contents, so it is not to be mounted + if not right_hand: location = None - mount_options = [] - # no contents, so it is not to be mounted - if not right_hand: - location = None - # just a string. per backward compatibility the mount point - elif isinstance(right_hand,str): - location = right_hand - # a dict. two elements 'mountpoint' (obvious) and and a mount options list ¿? - elif isinstance(right_hand,dict): - location = right_hand.get('mountpoint',None) - mount_options = right_hand.get('options',[]) - # we create the subvolume - create_subvolume(installation,name) - # Make the nodatacow processing now - # It will be the main cause of creation of subvolumes which are not to be mounted - # it is not an options which can be established by subvolume (but for whole file systems), and can be - # set up via a simple attribute change in a directory (if empty). And here the directories are brand new - if 'nodatacow' in mount_options: - if (cmd := SysCommand(f"chattr +C {installation.target}/{name}")).exit_code != 0: - raise DiskError(f"Could not set nodatacow attribute at {installation.target}/{name}: {cmd}") - # entry is deleted so nodatacow doesn't propagate to the mount options - del mount_options[mount_options.index('nodatacow')] - # Make the compress processing now - # it is not an options which can be established by subvolume (but for whole file systems), and can be - # set up via a simple attribute change in a directory (if empty). And here the directories are brand new - # in this way only zstd compression is activaded - # TODO WARNING it is not clear if it should be a standard feature, so it might need to be deactivated - if 'compress' in mount_options: - if (cmd := SysCommand(f"chattr +c {installation.target}/{name}")).exit_code != 0: - raise DiskError(f"Could not set compress attribute at {installation.target}/{name}: {cmd}") - # entry is deleted so nodatacow doesn't propagate to the mount options - del mount_options[mount_options.index('compress')] - # END compress processing. - # we do not mount if THE basic partition will be mounted or if we exclude explicitly this subvolume - if not partition['mountpoint'] and location is not None: - # we begin to create a fake partition entry. First we copy the original -the one that corresponds to - # the primary partition - fake_partition = partition.copy() - # we start to modify entries in the "fake partition" to match the needs of the subvolumes - # - # to avoid any chance of entering in a loop (not expected) we delete the list of subvolumes in the copy - # and reset the encryption parameters - del fake_partition['btrfs'] - fake_partition['encrypted'] = False - fake_partition['generate-encryption-key-file'] = False - # Mount destination. As of now the right hand part - fake_partition['mountpoint'] = location - # we load the name in an attribute called subvolume, but i think it is not needed anymore, 'cause the mount logic uses a different path. - fake_partition['subvolume'] = name - # here we add the mount options - fake_partition['options'] = mount_options - # Here comes the most exotic part. The dictionary attribute 'device_instance' contains an instance of Partition. This instance will be queried along the mount process at the installer. - # We instanciate a new object with following attributes coming / adapted from the instance which was in the primary partition entry (the one we are coping - partition['device_instance'] - # * path, which will be expanded with the subvolume name to use the bind mount syntax the system uses for naming mounted subvolumes - # * size. When the OS queries all the subvolumes share the same size as the full partititon - # * uuid. All the subvolumes on a partition share the same uuid - if not unlocked_device: - fake_partition['device_instance'] = Partition(f"{partition['device_instance'].path}[/{name}]",partition['device_instance'].size,partition['device_instance'].uuid) - else: - # for subvolumes IN an encrypted partition we make our device instance from unlocked device instead of the raw partition. - # This time we make a copy (we should to the same above TODO) and alter the path by hand - from copy import copy - # KIDS DONT'T DO THIS AT HOME - fake_partition['device_instance'] = copy(unlocked_device) - fake_partition['device_instance'].path = f"{unlocked_device.path}[/{name}]" - # we reset this attribute, which holds where the partition is actually mounted. Remember, the physical partition is mounted at this moment and therefore has the value '/'. - # If i don't reset it, process will abort as "already mounted' . - # TODO It works for this purpose, but the fact that this bevahiour can happed, should make think twice - fake_partition['device_instance'].mountpoint = None - # - # Well, now that this "fake partition" is ready, we add it to the list of the ones which are to be mounted, - # as "normal" ones - mountpoints[fake_partition['mountpoint']] = fake_partition - except Exception as e: - raise e - # if the physical partition has been selected to be mounted, we include it at the list. Remmeber, all the above treatement won't happen except the creation of the subvolume - if partition['mountpoint']: - mountpoints[partition['mountpoint']] = partition + + # just a string. per backward compatibility the mount point + elif isinstance(right_hand, str): + location = right_hand + # a dict. two elements 'mountpoint' (obvious) and and a mount options list ¿? + elif isinstance(right_hand, dict): + location = right_hand.get('mountpoint',None) + mount_options = right_hand.get('options', []) + + if not mount_options or any(['subvol=' in x for x in mount_options]) is False: + mount_options = [f'subvol={name}'] + + mountpoints[location] = {'partition': partition, 'mount_options' : mount_options} + + # we create the subvolume + create_subvolume(installation, name) + # Make the nodatacow processing now + # It will be the main cause of creation of subvolumes which are not to be mounted + # it is not an options which can be established by subvolume (but for whole file systems), and can be + # set up via a simple attribute change in a directory (if empty). And here the directories are brand new + if 'nodatacow' in mount_options: + if (cmd := SysCommand(f"chattr +C {installation.target}/{name}")).exit_code != 0: + raise DiskError(f"Could not set nodatacow attribute at {installation.target}/{name}: {cmd}") + # entry is deleted so nodatacow doesn't propagate to the mount options + del mount_options[mount_options.index('nodatacow')] + + # Make the compress processing now + # it is not an options which can be established by subvolume (but for whole file systems), and can be + # set up via a simple attribute change in a directory (if empty). And here the directories are brand new + # in this way only zstd compression is activaded + # TODO WARNING it is not clear if it should be a standard feature, so it might need to be deactivated + if 'compress' in mount_options: + if (cmd := SysCommand(f"chattr +c {installation.target}/{name}")).exit_code != 0: + raise DiskError(f"Could not set compress attribute at {installation.target}/{name}: {cmd}") + # entry is deleted so nodatacow doesn't propagate to the mount options + del mount_options[mount_options.index('compress')] + + # END compress processing. + # we do not mount if THE basic partition will be mounted or if we exclude explicitly this subvolume + if not partition['mountpoint'] and location is not None: + # we begin to create a fake partition entry. First we copy the original -the one that corresponds to + # the primary partition + fake_partition = partition.copy() + + # we start to modify entries in the "fake partition" to match the needs of the subvolumes + # + # to avoid any chance of entering in a loop (not expected) we delete the list of subvolumes in the copy + # and reset the encryption parameters + del fake_partition['btrfs'] + fake_partition['encrypted'] = False + fake_partition['generate-encryption-key-file'] = False + # Mount destination. As of now the right hand part + fake_partition['mountpoint'] = location + # we load the name in an attribute called subvolume, but i think it is not needed anymore, 'cause the mount logic uses a different path. + fake_partition['subvolume'] = name + # here we add the mount options + fake_partition['options'] = mount_options + + # Here comes the most exotic part. The dictionary attribute 'device_instance' contains an instance of Partition. This instance will be queried along the mount process at the installer. + # We instanciate a new object with following attributes coming / adapted from the instance which was in the primary partition entry (the one we are coping - partition['device_instance'] + # * path, which will be expanded with the subvolume name to use the bind mount syntax the system uses for naming mounted subvolumes + # * size. When the OS queries all the subvolumes share the same size as the full partititon + # * uuid. All the subvolumes on a partition share the same uuid + if not unlocked_device: + # Create fake instance with '[/@]' could probably be implemented with a Partition().subvolume_id = @/ instead and have it print in __repr__ + fake_partition['device_instance'] = Partition(f"{partition['device_instance'].path}[/{name}]", partition['device_instance'].size, partition['device_instance'].uuid) + else: + # for subvolumes IN an encrypted partition we make our device instance from unlocked device instead of the raw partition. + # This time we make a copy (we should to the same above TODO) and alter the path by hand + from copy import copy + # KIDS DONT'T DO THIS AT HOME + fake_partition['device_instance'] = copy(unlocked_device) + fake_partition['device_instance'].path = f"{unlocked_device.path}[/{name}]" + + # we reset this attribute, which holds where the partition is actually mounted. Remember, the physical partition is mounted at this moment and therefore has the value '/'. + # If i don't reset it, process will abort as "already mounted' . + # TODO It works for this purpose, but the fact that this bevahiour can happed, should make think twice + fake_partition['device_instance'].mountpoint = None + + # Well, now that this "fake partition" is ready, we add it to the list of the ones which are to be mounted, + # as "normal" ones + + if partition['mountpoint'] and partition.get('btrfs', {}).get('subvolumes', False) is False: + mountpoints[partition['mountpoint']] = {'partition': partition} + + return mountpoints \ No newline at end of file diff --git a/archinstall/lib/disk/filesystem.py b/archinstall/lib/disk/filesystem.py index 88c7818347..96f7dadba0 100644 --- a/archinstall/lib/disk/filesystem.py +++ b/archinstall/lib/disk/filesystem.py @@ -69,7 +69,6 @@ def load_layout(self, layout :dict): for partition in layout.get('partitions', []): # We don't want to re-add an existing partition (those containing a UUID already) if partition.get('format', False) and not partition.get('PARTUUID', None): - print("Adding partition....") partition['device_instance'] = self.add_partition(partition.get('type', 'primary'), start=partition.get('start', '1MiB'), # TODO: Revisit sane block starts (4MB for memorycards for instance) end=partition.get('size', '100%'), @@ -78,7 +77,7 @@ def load_layout(self, layout :dict): # print('Device instance:', partition['device_instance']) elif (partition_uuid := partition.get('PARTUUID')) and (partition_instance := self.blockdevice.get_partition(uuid=partition_uuid)): - print("Re-using partition_instance:", partition_instance) + log(f"[BETA] Re-using partition_instance: {partition_instance}", level=logging.WARNING, fg="yellow") partition['device_instance'] = partition_instance else: raise ValueError(f"{self}.load_layout() doesn't know how to continue without a new partition definition or a UUID ({partition.get('PARTUUID')}) on the device ({self.blockdevice.get_partition(uuid=partition.get('PARTUUID'))}).") @@ -227,7 +226,7 @@ def set_name(self, partition: int, name: str) -> bool: return self.parted(f'{self.blockdevice.device} name {partition + 1} "{name}"') == 0 def set(self, partition: int, string: str): - log(f"Setting {string} on (parted) partition index {partition+1}", level=logging.INFO) + log(f"Setting '{string}'' on (parted) partition index {partition+1}", level=logging.INFO) return self.parted(f'{self.blockdevice.device} set {partition + 1} {string}') == 0 def parted_mklabel(self, device: str, disk_label: str): diff --git a/archinstall/lib/disk/helpers.py b/archinstall/lib/disk/helpers.py index 2a2ab7440e..dda3f9dd3e 100644 --- a/archinstall/lib/disk/helpers.py +++ b/archinstall/lib/disk/helpers.py @@ -190,21 +190,32 @@ def get_partitions_in_use(mountpoint) -> list: # So first, we create the partition without a BlockDevice and carefully only use it to get .real_device # Note: doing print(partition) here will break because the above mentioned issue. - partition = Partition(target['source'], None, filesystem=target.get('fstype', None), mountpoint=target['target']) - partition = Partition(target['source'], partition.real_device, filesystem=target.get('fstype', None), mountpoint=target['target']) + # Note: depending if the partition is encrypted, different ammount of steps is required. + # hence the multiple stages to this monster. + partition = Partition(target['source'], None, filesystem=target.get('fstype', None), mountpoint=target['target'], auto_mount=False) + partition = Partition(target['source'], partition.real_device, filesystem=target.get('fstype', None), mountpoint=target['target'], auto_mount=False) + + if partition.real_device not in all_disks(): + # Trying to resolve partition -> blockdevice (This is a bit of a hack) + block_device_name = pathlib.Path(partition.real_device).stem + block_device_class_link = pathlib.Path(f"/sys/class/block/{block_device_name}") + if not block_device_class_link.is_symlink(): + raise ValueError(f"Could not locate blockdevice for partition: {block_device_class_link}") + block_device_class_path = block_device_class_link.readlink() + + partition = Partition(target['source'], BlockDevice(f"/dev/{block_device_class_path.parent.stem}"), filesystem=target.get('fstype', None), mountpoint=target['target'], auto_mount=False) # Once we have the real device (for instance /dev/nvme0n1p5) we can find the parent block device using - # (lsblk pkname lists both the partition and blockdevice, BD being the last entry) - result = SysCommand(f'lsblk -no pkname {partition.real_device}').decode().rstrip('\r\n').split('\r\n')[-1] + result = min([x for x in SysCommand(f'lsblk -no pkname {partition.real_device}').decode().rstrip('\r\n').split('\r\n') if len(x)], key=len) block_device = BlockDevice(f"/dev/{result}") # Once we figured the block device out, we can properly create the partition object - partition = Partition(target['source'], block_device, filesystem=target.get('fstype', None), mountpoint=target['target']) + partition = Partition(target['source'], block_device, filesystem=target.get('fstype', None), mountpoint=target['target'], auto_mount=False) mounts.append(partition) for child in target.get('children', []): - mounts.append(Partition(child['source'], block_device, filesystem=child.get('fstype', None), mountpoint=child['target'])) + mounts.append(Partition(child['source'], block_device, filesystem=child.get('fstype', None), mountpoint=child['target'], auto_mount=False)) return mounts diff --git a/archinstall/lib/disk/partition.py b/archinstall/lib/disk/partition.py index 2efc266d74..f42c54cd57 100644 --- a/archinstall/lib/disk/partition.py +++ b/archinstall/lib/disk/partition.py @@ -15,7 +15,7 @@ class Partition: - def __init__(self, path: str, block_device: BlockDevice, part_id=None, filesystem=None, mountpoint=None, encrypted=False, autodetect_filesystem=True): + def __init__(self, path: str, block_device: BlockDevice, part_id=None, filesystem=None, mountpoint=None, encrypted=False, autodetect_filesystem=True, auto_mount=True): if not part_id: part_id = os.path.basename(path) @@ -29,7 +29,7 @@ def __init__(self, path: str, block_device: BlockDevice, part_id=None, filesyste self.encrypted = encrypted self.allow_formatting = False - if mountpoint: + if mountpoint and auto_mount is True: self.mount(mountpoint) try: @@ -358,9 +358,18 @@ def find_parent_of(self, data, name, parent=None): if parent := self.find_parent_of(child, name, parent=data['name']): return parent - def mount(self, target, fs=None, options=''): - if not self.mountpoint: - log(f'Mounting {self} to {target}', level=logging.INFO) + def mount_options_has_subvolume(self, options): + return any(['subvol=' in x for x in options]) + + def mount(self, target, fs=None, options=[]): + if type(options) == str: + options = options.split(' ') + + log(f"Attempting to mount {self} to {target} using options {options}", level=logging.INFO) + + # Do not mount a partition that is already mounted, unless we're using subvolumes: + # TODO: This can be removed in favor of the new code in `master` at some point. + if not self.mountpoint or self.mount_options_has_subvolume(options): if not fs: if not self.filesystem: raise DiskError(f'Need to format (or define) the filesystem on {self} before mounting.') @@ -370,29 +379,30 @@ def mount(self, target, fs=None, options=''): pathlib.Path(target).mkdir(parents=True, exist_ok=True) + # If we're using bind_name (?) then append to the options if needed. if self.bind_name: device_path = self.device_path - # TODO options should be better be a list than a string - if options: - options = f"{options},subvol={self.bind_name}" - else: - options = f"subvol={self.bind_name}" + if not self.mount_options_has_subvolume(options): + options.append(f"subvol={self.bind_name}") else: device_path = self.path - try: - if options: - mnt_handle = SysCommand(f"/usr/bin/mount -t {fs_type} -o {options} {device_path} {target}") - else: - mnt_handle = SysCommand(f"/usr/bin/mount -t {fs_type} {device_path} {target}") - - # TODO: Should be redundant to check for exit_code - if mnt_handle.exit_code != 0: - raise DiskError(f"Could not mount {self.path} to {target} using options {options}") - except SysCallError as err: - raise err + + if options: + mount_options = f"-o {','.join(options)}" + else: + mount_options = '' + + log(f"Mount command: /usr/bin/mount -t {fs_type} {mount_options} {device_path} {target}", fg="yellow") + mnt_handle = SysCommand(f"/usr/bin/mount -t {fs_type} {mount_options} {device_path} {target}") + + # TODO: Should be redundant to check for exit_code + if mnt_handle.exit_code != 0: + raise DiskError(f"Could not mount {self.path} to {target} using options {options}: {mnt_handle}") self.mountpoint = target return True + else: + raise DiskError(f"Partition is already mounted to {self.mountpoint} but also with {self.bind_name}") def unmount(self): try: diff --git a/archinstall/lib/disk/user_guides.py b/archinstall/lib/disk/user_guides.py index c370a3a2a3..7f03a25632 100644 --- a/archinstall/lib/disk/user_guides.py +++ b/archinstall/lib/disk/user_guides.py @@ -39,7 +39,7 @@ def suggest_single_disk_layout(block_device, default_filesystem=None, advanced_o "start" : "518MB", "encrypted" : False, "format" : True, - "mountpoint" : "/", + "mountpoint" : '/' if not using_subvolumes else None, "filesystem" : { "format" : default_filesystem } @@ -81,7 +81,7 @@ def suggest_single_disk_layout(block_device, default_filesystem=None, advanced_o "format" : True, "start" : f"{min(block_device.size, MIN_SIZE_TO_ALLOW_HOME_PART)}GB", "size" : "100%", - "mountpoint" : "/home", + "mountpoint" : "/home" if not using_subvolumes else None, "filesystem" : { "format" : default_filesystem } diff --git a/archinstall/lib/installer.py b/archinstall/lib/installer.py index a14c4a95e4..5c14b49415 100644 --- a/archinstall/lib/installer.py +++ b/archinstall/lib/installer.py @@ -119,6 +119,8 @@ def __init__(self, target, *, base_packages=None, kernels=None): self.HOOKS = ["base", "udev", "autodetect", "keyboard", "keymap", "modconf", "block", "filesystems", "fsck"] self.KERNEL_PARAMS = [] + self.installed_packages = [] + def log(self, *args, level=logging.DEBUG, **kwargs): """ installer.log() wraps output.log() mainly to set a default log-level for this install session. @@ -197,29 +199,39 @@ def mount_ordered_layout(self, layouts: dict): # Immediately unlock the encrypted device to format the inner volume with luks2(partition['device_instance'], loopdev, partition['!password'], auto_unmount=False) as unlocked_device: unlocked_device.mount(f"{self.target}/") + try: - manage_btrfs_subvolumes(self,partition,mountpoints,subvolumes,unlocked_device) + mountpoints.update(manage_btrfs_subvolumes(self, partition, mountpoints, subvolumes, unlocked_device)) except Exception as e: # every exception unmounts the physical volume. Otherwise we let the system in an unstable state unlocked_device.unmount() raise e + unlocked_device.unmount() # TODO generate key else: self.mount(partition['device_instance'],"/") try: - manage_btrfs_subvolumes(self,partition,mountpoints,subvolumes) + mountpoints.update(manage_btrfs_subvolumes(self,partition,mountpoints,subvolumes)) except Exception as e: # every exception unmounts the physical volume. Otherwise we let the system in an unstable state partition['device_instance'].unmount() raise e partition['device_instance'].unmount() else: - mountpoints[partition['mountpoint']] = partition + mountpoints[partition['mountpoint']] = {'partition' : partition} + for mountpoint in sorted([mnt_dest for mnt_dest in mountpoints.keys() if mnt_dest is not None]): - partition = mountpoints[mountpoint] + partition = mountpoints[mountpoint]['partition'] + if partition.get('encrypted', False) and not partition.get('subvolume',None): - loopdev = f"{storage.get('ENC_IDENTIFIER', 'ai')}{pathlib.Path(partition['mountpoint']).name}loop" + if partition.get('mountpoint',None): + log(f"Encryption a partition that has no mountpoint target: {partition}", level=logging.WARNING, fg="yellow") + ppath = partition['mountpoint'] + else: + ppath = partition['device_instance'].path + + loopdev = f"{storage.get('ENC_IDENTIFIER', 'ai')}{pathlib.Path(ppath).name}loop" if not (password := partition.get('!password', None)): raise RequirementError(f"Missing mountpoint {mountpoint} encryption password in layout: {partition}") @@ -230,25 +242,30 @@ def mount_ordered_layout(self, layouts: dict): # Once we store the key as ../xyzloop.key systemd-cryptsetup can automatically load this key # if we name the device to "xyzloop". - encryption_key_path = f"/etc/cryptsetup-keys.d/{pathlib.Path(partition['mountpoint']).name}loop.key" - with open(f"{self.target}{encryption_key_path}", "w") as keyfile: - keyfile.write(generate_password(length=512)) + encryption_key_path = f"/etc/cryptsetup-keys.d/{pathlib.Path(ppath).name}loop.key" - os.chmod(f"{self.target}{encryption_key_path}", 0o400) + # Avoid creating the same encryption key file over and over when using subvolumes: + if not pathlib.Path(f"{self.target}{encryption_key_path}").exists(): + with open(f"{self.target}{encryption_key_path}", "w") as keyfile: + keyfile.write(generate_password(length=512)) - luks_handle.add_key(pathlib.Path(f"{self.target}{encryption_key_path}"), password=password) - luks_handle.crypttab(self, encryption_key_path, options=["luks", "key-slot=1"]) + os.chmod(f"{self.target}{encryption_key_path}", 0o400) + + luks_handle.add_key(pathlib.Path(f"{self.target}{encryption_key_path}"), password=password) + luks_handle.crypttab(self, encryption_key_path, options=["luks", "key-slot=1"]) log(f"Mounting {mountpoint} to {self.target}{mountpoint} using {unlocked_device}", level=logging.INFO) - unlocked_device.mount(f"{self.target}{mountpoint}") + unlocked_device.mount(f"{self.target}{mountpoint}", options=mountpoints[mountpoint].get('mount_options')) else: - log(f"Mounting {mountpoint} to {self.target}{mountpoint} using {partition['device_instance']}", level=logging.INFO) - if partition.get('options',[]): - mount_options = ','.join(partition['options']) - partition['device_instance'].mount(f"{self.target}{mountpoint}",options=mount_options) + if partition.get('options',[]) or mountpoints[mountpoint].get('mount_options', []): + mount_options = ','.join(partition.get('options', []) + mountpoints[mountpoint].get('mount_options', [])) else: - partition['device_instance'].mount(f"{self.target}{mountpoint}") + mount_options = None + + log(f"Mounting {partition['device_instance']} as {mountpoint} to {self.target}{mountpoint} using options {mount_options}", level=logging.INFO) + partition['device_instance'].mount(f"{self.target}{mountpoint}", options=mount_options) + time.sleep(1) try: get_mount_info(f"{self.target}{mountpoint}", traverse=False) @@ -277,6 +294,7 @@ def pacstrap(self, *packages, **kwargs): if (sync_mirrors := SysCommand('/usr/bin/pacman -Syy')).exit_code == 0: if (pacstrap := SysCommand(f'/usr/bin/pacstrap {self.target} {" ".join(packages)} --noconfirm', peak_output=True)).exit_code == 0: + self.installed_packages += packages return True else: self.log(f'Could not strap in packages: {pacstrap}', level=logging.ERROR, fg="red") @@ -628,14 +646,14 @@ def add_bootloader(self, bootloader='systemd-bootctl'): else: loader_data = [ f"default {self.init_time}", - "timeout 5" + "timeout 15" ] with open(f'{self.target}/boot/loader/loader.conf', 'w') as loader: for line in loader_data: if line[:8] == 'default ': loader.write(f'default {self.init_time}_{self.kernels[0]}\n') - elif line[:8] == '#timeout' and 'timeout 5' not in loader_data: + elif line[:8] == '#timeout' and 'timeout 15' not in loader_data: # We add in the default timeout to support dual-boot loader.write(f"{line[1:]}\n") else: @@ -667,9 +685,11 @@ def add_bootloader(self, bootloader='systemd-bootctl'): options_entry = f'rw intel_pstate=no_hwp rootfstype={root_fs_type} {" ".join(self.KERNEL_PARAMS)}\n' else: options_entry = f'rw intel_pstate=no_hwp {" ".join(self.KERNEL_PARAMS)}\n' - base_path,bind_path = split_bind_name(str(root_partition.path)) + + base_path, bind_path = split_bind_name(str(root_partition.path)) if bind_path is not None: # and root_fs_type == 'btrfs': options_entry = f"rootflags=subvol={bind_path} " + options_entry + if real_device := self.detect_encryption(root_partition): # TODO: We need to detect if the encrypted device is a whole disk encryption, # or simply a partition encryption. Right now we assume it's a partition (and we always have) @@ -682,26 +702,40 @@ def add_bootloader(self, bootloader='systemd-bootctl'): self.helper_flags['bootloader'] = bootloader elif bootloader == "grub-install": - self.pacstrap('grub') # no need? + if 'grub' not in self.installed_packages: + self.pacstrap('grub') + _file = "/etc/default/grub" + if real_device := self.detect_encryption(root_partition): root_uuid = SysCommand(f"blkid -s UUID -o value {real_device.path}").decode().rstrip() - _file = "/etc/default/grub" - add_to_CMDLINE_LINUX = f"sed -i 's/GRUB_CMDLINE_LINUX=\"\"/GRUB_CMDLINE_LINUX=\"cryptdevice=UUID={root_uuid}:cryptlvm rootfstype={root_fs_type}\"/'" + options_entry = f"cryptdevice=UUID={root_uuid}:cryptlvm rootfstype={root_fs_type}" + + # Extend the kernel options to support btrfs subvolumes if used + base_path, bind_path = split_bind_name(str(root_partition.path)) + if bind_path is not None: # and root_fs_type == 'btrfs': + options_entry = f"rootflags=subvol={bind_path} {options_entry}" + enable_CRYPTODISK = "sed -i 's/#GRUB_ENABLE_CRYPTODISK=y/GRUB_ENABLE_CRYPTODISK=y/'" log(f"Using UUID {root_uuid} of {real_device} as encrypted root identifier.", level=logging.INFO) - SysCommand(f"/usr/bin/arch-chroot {self.target} {add_to_CMDLINE_LINUX} {_file}") SysCommand(f"/usr/bin/arch-chroot {self.target} {enable_CRYPTODISK} {_file}") else: - _file = "/etc/default/grub" - add_to_CMDLINE_LINUX = f"sed -i 's/GRUB_CMDLINE_LINUX=\"\"/GRUB_CMDLINE_LINUX=\"rootfstype={root_fs_type}\"/'" - SysCommand(f"/usr/bin/arch-chroot {self.target} {add_to_CMDLINE_LINUX} {_file}") + options_entry = f"rootfstype={root_fs_type}" + + # Extend the kernel options to support btrfs subvolumes if used + base_path, bind_path = split_bind_name(str(root_partition.path)) + if bind_path is not None: # and root_fs_type == 'btrfs': + options_entry = f"rootflags=subvol={bind_path} {options_entry}" + + # TODO: Replace 'sed' with just file manipulation + add_to_CMDLINE_LINUX = f"sed -i 's%GRUB_CMDLINE_LINUX=\"\"%GRUB_CMDLINE_LINUX=\"{options_entry}\"%'" + SysCommand(f"/usr/bin/arch-chroot {self.target} {add_to_CMDLINE_LINUX} {_file}") log(f"GRUB uses {boot_partition.path} as the boot partition.", level=logging.INFO) if has_uefi(): self.pacstrap('efibootmgr') # TODO: Do we need? Yes, but remove from minimal_installation() instead? - if not (handle := SysCommand(f'/usr/bin/arch-chroot {self.target} grub-install --target=x86_64-efi --efi-directory=/boot --bootloader-id=GRUB')).exit_code == 0: + if not (handle := SysCommand(f'/usr/bin/arch-chroot {self.target} grub-install --target=x86_64-efi --efi-directory=/boot --bootloader-id=GRUB --removable')).exit_code == 0: raise DiskError(f"Could not install GRUB to {self.target}/boot: {handle}") else: if not (handle := SysCommand(f'/usr/bin/arch-chroot {self.target} grub-install --target=i386-pc --recheck {boot_partition.parent}')).exit_code == 0: diff --git a/archinstall/lib/systemd.py b/archinstall/lib/systemd.py index c3beafc0e0..47c5592dca 100644 --- a/archinstall/lib/systemd.py +++ b/archinstall/lib/systemd.py @@ -93,10 +93,11 @@ def __exit__(self, *args, **kwargs): while self.session.is_alive(): time.sleep(0.25) - if shutdown.exit_code == 0: + # Just one of them has to exit properly + if shutdown.exit_code == 0 or self.session.exit_code == 0: storage['active_boot'] = None else: - raise SysCallError(f"Could not shut down temporary boot of {self.instance}: {shutdown}", exit_code=shutdown.exit_code) + raise SysCallError(f"Could not shut down temporary boot of {self.instance} [{self.session.exit_code}/{shutdown.exit_code}]: {self.session.decode()[-200:]}", exit_code=shutdown.exit_code) def __iter__(self): if self.session: diff --git a/archinstall/lib/user_interaction.py b/archinstall/lib/user_interaction.py index 7d90915f4d..e20dd6d0e4 100644 --- a/archinstall/lib/user_interaction.py +++ b/archinstall/lib/user_interaction.py @@ -199,8 +199,9 @@ def select_encrypted_partitions(block_devices :dict, password :str) -> dict: partition['encrypted'] = True partition['!password'] = password - if partition['mountpoint'] != '/': + if partition['mountpoint'] and partition['mountpoint'] != '/': # Tell the upcoming steps to generate a key-file for non root mounts. + log(f"Marking partition for use with encryption-key: {partition}", level=logging.WARNING, fg="yellow") partition['generate-encryption-key-file'] = True return block_devices