From 3531896892da84a60723fb98ca1d58805505d290 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 6 Jul 2021 15:31:00 +0200 Subject: [PATCH 1/7] [mount] Factor out the default btrfs configuration --- src/modules/mount/main.py | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/src/modules/mount/main.py b/src/modules/mount/main.py index 5e5233935..8f019f1a3 100644 --- a/src/modules/mount/main.py +++ b/src/modules/mount/main.py @@ -29,6 +29,24 @@ _ = gettext.translation("calamares-python", def pretty_name(): return _("Mounting partitions.") + +def get_btrfs_subvolumes(): + """ + Gets the job-configuration for btrfs subvolumes, or if there is + none given, returns a default configuration that matches + the setup (/ and /home) from before configurability was introduced. + """ + btrfs_subvolumes = libcalamares.job.configuration.get("btrfsSubvolumes", None) + # Warn if there's no configuration at all, and empty configurations are + # replaced by a simple root-only layout. + if btrfs_subvolumes is None: + libcalamares.utils.warning("No configuration for btrfsSubvolumes") + if not btrfs_subvolumes: + btrfs_subvolumes = [ dict(mountPoint="/", subvolume="/@") ] + + return btrfs_subvolumes + + def mount_partition(root_mount_point, partition, partitions): """ Do a single mount of @p partition inside @p root_mount_point. @@ -74,13 +92,7 @@ def mount_partition(root_mount_point, partition, partitions): # Special handling for btrfs subvolumes. Create the subvolumes listed in mount.conf if fstype == "btrfs" and partition["mountPoint"] == '/': # Root has been mounted to btrfs volume -> create subvolumes from configuration - btrfs_subvolumes = libcalamares.job.configuration.get("btrfsSubvolumes", None) - # Warn if there's no configuration at all, and empty configurations are - # replaced by a simple root-only layout. - if btrfs_subvolumes is None: - libcalamares.utils.warning("No configuration for btrfsSubvolumes") - if not btrfs_subvolumes: - btrfs_subvolumes = [ dict(mountPoint="/", subvolume="/@") ] + btrfs_subvolumes = get_btrfs_subvolumes() subvolume_mountpoints = [d['mountPoint'] for d in btrfs_subvolumes] # Check if listed mountpoints besides / are already present and don't create subvolume for those From 6b2088c94e72d739862606060d48ba122bcb54c4 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 6 Jul 2021 15:34:13 +0200 Subject: [PATCH 2/7] [mount] Restore @home subvolume In 942221c764a8622c9b23d29b1db291404af0f63b the fixed-setup (with /@ and /@home) was replaced by the configurable btrfs layout, but the default went away. Restore the two-subvolume layout if nothing is configured. --- src/modules/mount/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/mount/main.py b/src/modules/mount/main.py index 8f019f1a3..50eba72cb 100644 --- a/src/modules/mount/main.py +++ b/src/modules/mount/main.py @@ -42,7 +42,7 @@ def get_btrfs_subvolumes(): if btrfs_subvolumes is None: libcalamares.utils.warning("No configuration for btrfsSubvolumes") if not btrfs_subvolumes: - btrfs_subvolumes = [ dict(mountPoint="/", subvolume="/@") ] + btrfs_subvolumes = [ dict(mountPoint="/", subvolume="/@"), dict(mountPoint="/home", subvolume="/@home") ] return btrfs_subvolumes From cc357140e569d7ea9293d1ceb6a73a51dd491518 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 6 Jul 2021 16:43:56 +0200 Subject: [PATCH 3/7] [mount] Factor out the subvolume-filtering for partitions --- src/modules/mount/main.py | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/modules/mount/main.py b/src/modules/mount/main.py index 50eba72cb..1a97718b0 100644 --- a/src/modules/mount/main.py +++ b/src/modules/mount/main.py @@ -30,11 +30,16 @@ def pretty_name(): return _("Mounting partitions.") -def get_btrfs_subvolumes(): +def get_btrfs_subvolumes(partitions): """ Gets the job-configuration for btrfs subvolumes, or if there is none given, returns a default configuration that matches the setup (/ and /home) from before configurability was introduced. + + @param partitions + The partitions (from the partitioning module) that will exist on disk. + This is used to filter out subvolumes that don't need to be created + because they get a dedicated partition instead. """ btrfs_subvolumes = libcalamares.job.configuration.get("btrfsSubvolumes", None) # Warn if there's no configuration at all, and empty configurations are @@ -44,6 +49,10 @@ def get_btrfs_subvolumes(): if not btrfs_subvolumes: btrfs_subvolumes = [ dict(mountPoint="/", subvolume="/@"), dict(mountPoint="/home", subvolume="/@home") ] + # Filter out the subvolumes which have a dedicated partition + non_root_partition_mounts = [ m for m in [ p.get("mountPoint", None) for p in partitions ] if m is not None and m != '/' ] + btrfs_subvolumes = filter(lambda s : s["mountPoint"] in non_root_partition_mounts, btrfs_subvolumes) + return btrfs_subvolumes @@ -92,15 +101,8 @@ def mount_partition(root_mount_point, partition, partitions): # Special handling for btrfs subvolumes. Create the subvolumes listed in mount.conf if fstype == "btrfs" and partition["mountPoint"] == '/': # Root has been mounted to btrfs volume -> create subvolumes from configuration - btrfs_subvolumes = get_btrfs_subvolumes() + btrfs_subvolumes = get_btrfs_subvolumes(partitions) - subvolume_mountpoints = [d['mountPoint'] for d in btrfs_subvolumes] - # Check if listed mountpoints besides / are already present and don't create subvolume for those - for p in partitions: - if "mountPoint" not in p or not p["mountPoint"]: - continue - if p["mountPoint"] in subvolume_mountpoints and p["mountPoint"] != '/': - btrfs_subvolumes = [d for d in btrfs_subvolumes if d['mountPoint'] != p["mountPoint"]] # Check if we need a subvolume for swap file swap_choice = global_storage.value( "partitionChoices" ) if swap_choice: From e800b2da2d35c6720973fc0437c6160340baf577 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 6 Jul 2021 16:48:46 +0200 Subject: [PATCH 4/7] [mount] Factor out swap-subvolume setting --- src/modules/mount/main.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/modules/mount/main.py b/src/modules/mount/main.py index 1a97718b0..f5faad7d1 100644 --- a/src/modules/mount/main.py +++ b/src/modules/mount/main.py @@ -53,6 +53,11 @@ def get_btrfs_subvolumes(partitions): non_root_partition_mounts = [ m for m in [ p.get("mountPoint", None) for p in partitions ] if m is not None and m != '/' ] btrfs_subvolumes = filter(lambda s : s["mountPoint"] in non_root_partition_mounts, btrfs_subvolumes) + # If we have a swap **file**, give it a separate subvolume. + swap_choice = libcalamares.globalstorage.value( "partitionChoices" ) + if swap_choice and swap_choice.get( "swap", None ) == "file": + btrfs_subvolumes.append({'mountPoint': '/swap', 'subvolume': '/@swap'}) + return btrfs_subvolumes @@ -62,7 +67,6 @@ def mount_partition(root_mount_point, partition, partitions): """ # Create mount point with `+` rather than `os.path.join()` because # `partition["mountPoint"]` starts with a '/'. - global_storage = libcalamares.globalstorage raw_mount_point = partition["mountPoint"] if not raw_mount_point: return @@ -103,12 +107,6 @@ def mount_partition(root_mount_point, partition, partitions): # Root has been mounted to btrfs volume -> create subvolumes from configuration btrfs_subvolumes = get_btrfs_subvolumes(partitions) - # Check if we need a subvolume for swap file - swap_choice = global_storage.value( "partitionChoices" ) - if swap_choice: - swap_choice = swap_choice.get( "swap", None ) - if swap_choice and swap_choice == "file": - btrfs_subvolumes.append({'mountPoint': '/swap', 'subvolume': '/@swap'}) # Store created list in global storage so it can be used in the fstab module libcalamares.globalstorage.insert("btrfsSubvolumes", btrfs_subvolumes) # Create the subvolumes that are in the completed list From 995646936ff357bdaeab1d3f297c40b607823ddc Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 7 Jul 2021 12:38:41 +0200 Subject: [PATCH 5/7] [mount] Add test exercising refactored btrfs code --- src/modules/mount/tests/3.global | 8 ++++++++ src/modules/mount/tests/3.job | 6 ++++++ 2 files changed, 14 insertions(+) create mode 100644 src/modules/mount/tests/3.global create mode 100644 src/modules/mount/tests/3.job diff --git a/src/modules/mount/tests/3.global b/src/modules/mount/tests/3.global new file mode 100644 index 000000000..9dae421df --- /dev/null +++ b/src/modules/mount/tests/3.global @@ -0,0 +1,8 @@ +# SPDX-FileCopyrightText: no +# SPDX-License-Identifier: CC0-1.0 +partitions: + - device: "/dev/sdb1" + mountPoint: "/" + fs: "btrfs" +partitionChoices: + swap: file diff --git a/src/modules/mount/tests/3.job b/src/modules/mount/tests/3.job new file mode 100644 index 000000000..94b3a1492 --- /dev/null +++ b/src/modules/mount/tests/3.job @@ -0,0 +1,6 @@ +# SPDX-FileCopyrightText: no +# SPDX-License-Identifier: CC0-1.0 +bogus: true + +# No configuration needed because the partitions are +# all filesystems that require no special handling. From eb4ffe737e55240759ff69c2ffb0108bb4a52d47 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 7 Jul 2021 12:57:08 +0200 Subject: [PATCH 6/7] [mount] Fix logic - filter() returns the items for which the predicate is True; we want to keep the subvolumes that do not have an explicit partition already associated. - need list() to hammer it back into a list for appending swap subvol. --- src/modules/mount/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/mount/main.py b/src/modules/mount/main.py index f5faad7d1..2e96b6036 100644 --- a/src/modules/mount/main.py +++ b/src/modules/mount/main.py @@ -51,7 +51,7 @@ def get_btrfs_subvolumes(partitions): # Filter out the subvolumes which have a dedicated partition non_root_partition_mounts = [ m for m in [ p.get("mountPoint", None) for p in partitions ] if m is not None and m != '/' ] - btrfs_subvolumes = filter(lambda s : s["mountPoint"] in non_root_partition_mounts, btrfs_subvolumes) + btrfs_subvolumes = list(filter(lambda s : s["mountPoint"] not in non_root_partition_mounts, btrfs_subvolumes)) # If we have a swap **file**, give it a separate subvolume. swap_choice = libcalamares.globalstorage.value( "partitionChoices" ) From 6b9a1530f82f2be9477ba9e4fe63c6daa4c35d8f Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 7 Jul 2021 13:08:06 +0200 Subject: [PATCH 7/7] [mount] Add test exercising partial-filtering --- src/modules/mount/tests/4.global | 11 +++++++++++ src/modules/mount/tests/4.job | 12 ++++++++++++ 2 files changed, 23 insertions(+) create mode 100644 src/modules/mount/tests/4.global create mode 100644 src/modules/mount/tests/4.job diff --git a/src/modules/mount/tests/4.global b/src/modules/mount/tests/4.global new file mode 100644 index 000000000..1856c9dc3 --- /dev/null +++ b/src/modules/mount/tests/4.global @@ -0,0 +1,11 @@ +# SPDX-FileCopyrightText: no +# SPDX-License-Identifier: CC0-1.0 +partitions: + - device: "/dev/sdb1" + mountPoint: "/" + fs: "btrfs" + - device: "/dev/sdb2" + mountPoint: "/home" + fs: "ext4" +partitionChoices: + swap: file diff --git a/src/modules/mount/tests/4.job b/src/modules/mount/tests/4.job new file mode 100644 index 000000000..dac758227 --- /dev/null +++ b/src/modules/mount/tests/4.job @@ -0,0 +1,12 @@ +# SPDX-FileCopyrightText: no +# SPDX-License-Identifier: CC0-1.0 + +btrfsSubvolumes: + - mountPoint: / + subvolume: /@ + - mountPoint: /home + subvolume: /@home + - mountPoint: /var/cache + subvolume: /@cache + - mountPoint: /var/log + subvolume: /@log