Skip to content

Commit

Permalink
Fixing 781 and it's btrfs mountpoints and kernel parameters (#905)
Browse files Browse the repository at this point in the history 
* 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 <[email protected]>
  • Loading branch information
@Torxed @OneLongneck
Torxed and OneLongneck authored Jan 26, 2022
1 parent 60aa6c8 commit aa3b79c
Show file tree
Hide file tree
Showing 9 changed files with 221 additions and 150 deletions.
11 changes: 7 additions & 4 deletions archinstall/lib/disk/blockdevice.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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}")
178 changes: 95 additions & 83 deletions archinstall/lib/disk/btrfs.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 2 additions & 3 deletions archinstall/lib/disk/filesystem.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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%'),
Expand All @@ -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'))}).")
Expand Down Expand Up @@ -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):
Expand Down
23 changes: 17 additions & 6 deletions archinstall/lib/disk/helpers.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Loading

0 comments on commit aa3b79c

Please sign in to comment.