Refactoring and cleaning up a bit

I'm making sure that the JSON structure of the user config can get a say in how the subvolumes should be used later on. As well as splitting up where the logic should be to make it easier to maintain.

3 files changed, 24 insertions, 23 deletions

diff --git a/archinstall/lib/disk.py b/archinstall/lib/disk.py
index be5b9edb..740fd4ce 100644
--- a/archinstall/lib/disk.py
+++ b/archinstall/lib/disk.py
@@ -111,19 +111,6 @@ def select_disk_larger_than_or_close_to(devices, gigabytes, filter_out=None):
 
 	return min(copy_devices, key=(lambda device : abs(device.size - gigabytes)))
 
-def disk_layout_filesystem_checks(layout):
-	# This can probably be compressed into a any(<list comprehension>)
-	options = {}
-	for block_device in layout:
-		for partition in block_device.get('partitions', []):
-			if partition.get('filesystem', {}).get('format', False) == 'btrfs':
-				if not partition['filesystem'].get('subvolume', None):
-					if not options.get('btrfs-subvolumes', None) is None:
-						options['btrfs-subvolumes'] = input('Would you like to use BTRFS subvolumes? (Y/n)').strip().lower() in ('', 'y', 'yes')
-
-					if options['btrfs-subvolumes']:
-						btrfs.create_subvolume(partition)
-
 def suggest_single_disk_layout(block_device, default_filesystem=None):
 	if not default_filesystem:
 		from .user_interaction import ask_for_main_filesystem_format
@@ -164,7 +151,23 @@ def suggest_single_disk_layout(block_device, default_filesystem=None):
 		}
 	})
 
-	if block_device.size >= MIN_SIZE_TO_ALLOW_HOME_PART:
+	if default_filesystem == 'btrfs' and input('Would you like to use BTRFS subvolumes? (Y/n)').strip().lower() in ('', 'y', 'yes'):
+		# https://btrfs.wiki.kernel.org/index.php/FAQ
+		# https://unix.stackexchange.com/questions/246976/btrfs-subvolume-uuid-clash
+		# https://github.com/classy-giraffe/easy-arch/blob/main/easy-arch.sh
+		layout[block_device.path]['partitions'][1]['btrfs'] = {
+			"subvolumes" : {
+				'@home' : '/home',
+				'@log' : '/var/log',
+				'@pkgs' : '/var/cache/pacman/pkg',
+				'@.snapshots' : '/.snapshots'
+			}
+		}
+
+	elif block_device.size >= MIN_SIZE_TO_ALLOW_HOME_PART:
+		# If we don't want to use subvolumes,
+		# But we want to be able to re-use data between re-installs..
+		# A second partition for /home would be nice if we have the space for it
 		layout[block_device.path]['partitions'].append({
 			# Home
 			"type" : "primary",
@@ -186,6 +189,10 @@ def suggest_multi_disk_layout(block_devices, default_filesystem=None):
 		from .user_interaction import ask_for_main_filesystem_format
 		default_filesystem = ask_for_main_filesystem_format()
 
+	# Not really a rock solid foundation of information to stand on, but it's a start:
+	# https://www.reddit.com/r/btrfs/comments/m287gp/partition_strategy_for_two_physical_disks/
+	# https://www.reddit.com/r/btrfs/comments/9us4hr/what_is_your_btrfs_partitionsubvolumes_scheme/
+
 	MIN_SIZE_TO_ALLOW_HOME_PART = 40 # Gb
 	ARCH_LINUX_INSTALLED_SIZE = 20 # Gb, rough estimate taking in to account user desktops etc. TODO: Catch user packages to detect size?
 
diff --git a/archinstall/lib/disk2/btrfs.py b/archinstall/lib/disk2/btrfs.py
index 549d23c1..d6758b3f 100644
--- a/archinstall/lib/disk2/btrfs.py
+++ b/archinstall/lib/disk2/btrfs.py
@@ -1,9 +1,4 @@
-def create_subvolume(partition):
-	if partition['mountpoint'] == '/':
-		partition['filesystem']['subvolume'] = '@'
-	elif partition['mountpoint'] == '/home':
-		partition['filesystem']['subvolume'] = '@home'
+from ..general import SysCommand
 
-	# @.snapshots /.snapshots
-	# @log /var/log
-	# @pkg /var/cache/pacman/pkg
\ No newline at end of file
+def create_subvolume(installation):
+	SysCommand(f"btrfs subvolume create {installation.target}/@")
\ No newline at end of file
diff --git a/examples/guided.py b/examples/guided.py
index afe648e7..b7c75b30 100644
--- a/examples/guided.py
+++ b/examples/guided.py
@@ -116,7 +116,6 @@ def ask_user_questions():
 
 	if archinstall.arguments.get('harddrives', None) is not None and archinstall.storage.get('disk_layouts', None) is None:
 		archinstall.storage['disk_layouts'] = archinstall.select_disk_layout(archinstall.arguments['harddrives'])
-	archinstall.disk_layout_filesystem_checks(archinstall.storage['disk_layouts'])
 
 	# Get disk encryption password (or skip if blank)
 	if archinstall.arguments['harddrives'] and archinstall.arguments.get('!encryption-password', None) is None: