Fix Btrfs mount options (#2404)

6 files changed, 91 insertions, 124 deletions

diff --git a/archinstall/lib/disk/device_handler.py b/archinstall/lib/disk/device_handler.py
index 59ee150d..c06247e6 100644
--- a/archinstall/lib/disk/device_handler.py
+++ b/archinstall/lib/disk/device_handler.py
@@ -437,9 +437,19 @@ class DeviceHandler(object):
 			if not luks_handler.mapper_dev:
 				raise DiskError('Failed to unlock luks device')
 
-			self.mount(luks_handler.mapper_dev, self._TMP_BTRFS_MOUNT, create_target_mountpoint=True)
+			self.mount(
+				luks_handler.mapper_dev,
+				self._TMP_BTRFS_MOUNT,
+				create_target_mountpoint=True,
+				options=part_mod.mount_options
+			)
 		else:
-			self.mount(part_mod.safe_dev_path, self._TMP_BTRFS_MOUNT, create_target_mountpoint=True)
+			self.mount(
+				part_mod.safe_dev_path,
+				self._TMP_BTRFS_MOUNT,
+				create_target_mountpoint=True,
+				options=part_mod.mount_options
+			)
 
 		for sub_vol in part_mod.btrfs_subvols:
 			debug(f'Creating subvolume: {sub_vol.name}')
@@ -451,18 +461,6 @@ class DeviceHandler(object):
 
 			SysCommand(f"btrfs subvolume create {subvol_path}")
 
-			if sub_vol.nodatacow:
-				try:
-					SysCommand(f'chattr +C {subvol_path}')
-				except SysCallError as err:
-					raise DiskError(f'Could not set nodatacow attribute at {subvol_path}: {err}')
-
-			if sub_vol.compress:
-				try:
-					SysCommand(f'chattr +c {subvol_path}')
-				except SysCallError as err:
-					raise DiskError(f'Could not set compress attribute at {subvol_path}: {err}')
-
 		if luks_handler is not None and luks_handler.mapper_dev is not None:
 			self.umount(luks_handler.mapper_dev)
 			luks_handler.lock()
diff --git a/archinstall/lib/disk/device_model.py b/archinstall/lib/disk/device_model.py
index d4563faa..423c65e4 100644
--- a/archinstall/lib/disk/device_model.py
+++ b/archinstall/lib/disk/device_model.py
@@ -315,6 +315,11 @@ class Size:
 		return self._normalize() >= other._normalize()
 
 
+class BtrfsMountOption(Enum):
+	compress = 'compress=zstd'
+	nodatacow = 'nodatacow'
+
+
 @dataclass
 class _BtrfsSubvolumeInfo:
 	name: Path
@@ -458,8 +463,6 @@ class _DeviceInfo:
 class SubvolumeModification:
 	name: Path
 	mountpoint: Optional[Path] = None
-	compress: bool = False
-	nodatacow: bool = False
 
 	@classmethod
 	def from_existing_subvol_info(cls, info: _BtrfsSubvolumeInfo) -> SubvolumeModification:
@@ -475,31 +478,11 @@ class SubvolumeModification:
 
 			mountpoint = Path(entry['mountpoint']) if entry['mountpoint'] else None
 
-			compress = entry.get('compress', False)
-			nodatacow = entry.get('nodatacow', False)
-
-			if compress and nodatacow:
-				raise ValueError('compress and nodatacow flags cannot be enabled simultaneously on a btfrs subvolume')
-
-			mods.append(
-				SubvolumeModification(
-					entry['name'],
-					mountpoint,
-					compress,
-					nodatacow
-				)
-			)
+			mods.append(SubvolumeModification(entry['name'], mountpoint))
 
 		return mods
 
 	@property
-	def mount_options(self) -> List[str]:
-		options = []
-		options += ['compress'] if self.compress else []
-		options += ['nodatacow'] if self.nodatacow else []
-		return options
-
-	@property
 	def relative_mountpoint(self) -> Path:
 		"""
 		Will return the relative path based on the anchor
@@ -516,12 +499,7 @@ class SubvolumeModification:
 		return False
 
 	def json(self) -> Dict[str, Any]:
-		return {
-			'name': str(self.name),
-			'mountpoint': str(self.mountpoint),
-			'compress': self.compress,
-			'nodatacow': self.nodatacow
-		}
+		return {'name': str(self.name), 'mountpoint': str(self.mountpoint)}
 
 	def table_data(self) -> Dict[str, Any]:
 		return self.json()
diff --git a/archinstall/lib/disk/partitioning_menu.py b/archinstall/lib/disk/partitioning_menu.py
index a9478158..823605e3 100644
--- a/archinstall/lib/disk/partitioning_menu.py
+++ b/archinstall/lib/disk/partitioning_menu.py
@@ -5,7 +5,7 @@ from pathlib import Path
 from typing import Any, Dict, TYPE_CHECKING, List, Optional, Tuple
 
 from .device_model import PartitionModification, FilesystemType, BDevice, Size, Unit, PartitionType, PartitionFlag, \
-	ModificationStatus, DeviceGeometry, SectorSize
+	ModificationStatus, DeviceGeometry, SectorSize, BtrfsMountOption
 from ..hardware import SysInfo
 from ..menu import Menu, ListManager, MenuSelection, TextInput
 from ..output import FormattedOutput, warn
@@ -30,6 +30,7 @@ class PartitioningList(ListManager):
 			'mark_bootable': str(_('Mark/Unmark as bootable')),
 			'set_filesystem': str(_('Change filesystem')),
 			'btrfs_mark_compressed': str(_('Mark/Unmark as compressed')),  # btrfs only
+			'btrfs_mark_nodatacow': str(_('Mark/Unmark as nodatacow')),  # btrfs only
 			'btrfs_set_subvolumes': str(_('Set subvolumes')),  # btrfs only
 			'delete_partition': str(_('Delete partition'))
 		}
@@ -71,12 +72,17 @@ class PartitioningList(ListManager):
 				self._actions['set_filesystem'],
 				self._actions['mark_bootable'],
 				self._actions['btrfs_mark_compressed'],
+				self._actions['btrfs_mark_nodatacow'],
 				self._actions['btrfs_set_subvolumes']
 			]
 
 		# non btrfs partitions shouldn't get btrfs options
 		if selection.fs_type != FilesystemType.Btrfs:
-			not_filter += [self._actions['btrfs_mark_compressed'], self._actions['btrfs_set_subvolumes']]
+			not_filter += [
+				self._actions['btrfs_mark_compressed'],
+				self._actions['btrfs_mark_nodatacow'],
+				self._actions['btrfs_set_subvolumes']
+			]
 		else:
 			not_filter += [self._actions['assign_mountpoint']]
 
@@ -122,7 +128,9 @@ class PartitioningList(ListManager):
 					if fs_type == FilesystemType.Btrfs:
 						entry.mountpoint = None
 			case 'btrfs_mark_compressed' if entry:
-				self._set_compressed(entry)
+				self._toggle_mount_option(entry, BtrfsMountOption.compress)
+			case 'btrfs_mark_nodatacow' if entry:
+				self._toggle_mount_option(entry, BtrfsMountOption.nodatacow)
 			case 'btrfs_set_subvolumes' if entry:
 				self._set_btrfs_subvolumes(entry)
 			case 'delete_partition' if entry:
@@ -141,13 +149,28 @@ class PartitioningList(ListManager):
 		else:
 			return [d for d in data if d != entry]
 
-	def _set_compressed(self, partition: PartitionModification):
-		compression = 'compress=zstd'
+	def _toggle_mount_option(
+		self,
+		partition: PartitionModification,
+		option: BtrfsMountOption
+	):
+		if option.value not in partition.mount_options:
+			if option == BtrfsMountOption.compress:
+				partition.mount_options = [
+					o for o in partition.mount_options
+					if o != BtrfsMountOption.nodatacow.value
+				]
+
+			partition.mount_options = [
+				o for o in partition.mount_options
+				if not o.startswith(BtrfsMountOption.compress.name)
+			]
 
-		if compression in partition.mount_options:
-			partition.mount_options = [o for o in partition.mount_options if o != compression]
+			partition.mount_options.append(option.value)
 		else:
-			partition.mount_options.append(compression)
+			partition.mount_options = [
+				o for o in partition.mount_options if o != option.value
+			]
 
 	def _set_btrfs_subvolumes(self, partition: PartitionModification):
 		partition.btrfs_subvols = SubvolumeMenu(
diff --git a/archinstall/lib/disk/subvolume_menu.py b/archinstall/lib/disk/subvolume_menu.py
index 2b70d7b2..48afa829 100644
--- a/archinstall/lib/disk/subvolume_menu.py
+++ b/archinstall/lib/disk/subvolume_menu.py
@@ -2,7 +2,7 @@ from pathlib import Path
 from typing import Dict, List, Optional, Any, TYPE_CHECKING
 
 from .device_model import SubvolumeModification
-from ..menu import Menu, TextInput, MenuSelectionType, ListManager
+from ..menu import TextInput, ListManager
 from ..output import FormattedOutput
 
 if TYPE_CHECKING:
@@ -36,23 +36,6 @@ class SubvolumeMenu(ListManager):
 	def selected_action_display(self, subvolume: SubvolumeModification) -> str:
 		return str(subvolume.name)
 
-	def _prompt_options(self, editing: Optional[SubvolumeModification] = None) -> List[str]:
-		preset_options = []
-		if editing:
-			preset_options = editing.mount_options
-
-		choice = Menu(
-			str(_("Select the desired subvolume options ")),
-			['nodatacow', 'compress'],
-			skip=True,
-			preset_values=preset_options,
-		).run()
-
-		if choice.type_ == MenuSelectionType.Selection:
-			return choice.value  # type: ignore
-
-		return []
-
 	def _add_subvolume(self, editing: Optional[SubvolumeModification] = None) -> Optional[SubvolumeModification]:
 		name = TextInput(f'\n\n{_("Subvolume name")}: ', editing.name if editing else '').run()
 
@@ -64,13 +47,7 @@ class SubvolumeMenu(ListManager):
 		if not mountpoint:
 			return None
 
-		options = self._prompt_options(editing)
-
-		subvolume = SubvolumeModification(Path(name), Path(mountpoint))
-		subvolume.compress = 'compress' in options
-		subvolume.nodatacow = 'nodatacow' in options
-
-		return subvolume
+		return SubvolumeModification(Path(name), Path(mountpoint))
 
 	def handle_action(
 		self,
diff --git a/archinstall/lib/installer.py b/archinstall/lib/installer.py
index d5ea889b..c53e922d 100644
--- a/archinstall/lib/installer.py
+++ b/archinstall/lib/installer.py
@@ -240,7 +240,11 @@ class Installer:
 			disk.device_handler.mount(part_mod.dev_path, target, options=part_mod.mount_options)
 
 		if part_mod.fs_type == disk.FilesystemType.Btrfs and part_mod.dev_path:
-			self._mount_btrfs_subvol(part_mod.dev_path, part_mod.btrfs_subvols)
+			self._mount_btrfs_subvol(
+				part_mod.dev_path,
+				part_mod.btrfs_subvols,
+				part_mod.mount_options
+			)
 
 	def _mount_luks_partition(self, part_mod: disk.PartitionModification, luks_handler: Luks2):
 		# it would be none if it's btrfs as the subvolumes will have the mountpoints defined
@@ -251,11 +255,18 @@ class Installer:
 		if part_mod.fs_type == disk.FilesystemType.Btrfs and luks_handler.mapper_dev:
 			self._mount_btrfs_subvol(luks_handler.mapper_dev, part_mod.btrfs_subvols)
 
-	def _mount_btrfs_subvol(self, dev_path: Path, subvolumes: List[disk.SubvolumeModification]):
+	def _mount_btrfs_subvol(
+		self,
+		dev_path: Path,
+		subvolumes: List[disk.SubvolumeModification],
+		mount_options: List[str] = []
+	):
 		for subvol in subvolumes:
-			mountpoint = self.target / subvol.relative_mountpoint
-			mount_options = subvol.mount_options + [f'subvol={subvol.name}']
-			disk.device_handler.mount(dev_path, mountpoint, options=mount_options)
+			disk.device_handler.mount(
+				dev_path,
+				self.target / subvol.relative_mountpoint,
+				options=mount_options + [f'subvol={subvol.name}']
+			)
 
 	def generate_key_files(self):
 		for part_mod in self._disk_encryption.partitions:
@@ -382,37 +393,6 @@ class Installer:
 			for entry in self._fstab_entries:
 				fp.write(f'{entry}\n')
 
-		for mod in self._disk_config.device_modifications:
-			for part_mod in mod.partitions:
-				if part_mod.fs_type != disk.FilesystemType.Btrfs:
-					continue
-
-				with fstab_path.open('r') as fp:
-					fstab = fp.readlines()
-
-				# Replace the {installation}/etc/fstab with entries
-				# using the compress=zstd where the mountpoint has compression set.
-				for index, line in enumerate(fstab):
-					# So first we grab the mount options by using subvol=.*? as a locator.
-					# And we also grab the mountpoint for the entry, for instance /var/log
-					subvoldef = re.findall(',.*?subvol=.*?[\t ]', line)
-					mountpoint = re.findall('[\t ]/.*?[\t ]', line)
-
-					if not subvoldef or not mountpoint:
-						continue
-
-					for sub_vol in part_mod.btrfs_subvols:
-						# We then locate the correct subvolume and check if it's compressed,
-						# and skip entries where compression is already defined
-						# We then sneak in the compress=zstd option if it doesn't already exist:
-						if sub_vol.compress and str(sub_vol.mountpoint) == Path(
-							mountpoint[0].strip()) and ',compress=zstd,' not in line:
-							fstab[index] = line.replace(subvoldef[0], f',compress=zstd{subvoldef[0]}')
-							break
-
-				with fstab_path.open('w') as fp:
-					fp.writelines(fstab)
-
 	def set_hostname(self, hostname: str, *args: str, **kwargs: str) -> None:
 		with open(f'{self.target}/etc/hostname', 'w') as fh:
 			fh.write(hostname + '\n')
diff --git a/archinstall/lib/interactions/disk_conf.py b/archinstall/lib/interactions/disk_conf.py
index 72a32311..9d0042d6 100644
--- a/archinstall/lib/interactions/disk_conf.py
+++ b/archinstall/lib/interactions/disk_conf.py
@@ -5,6 +5,7 @@ from typing import Any, TYPE_CHECKING
 from typing import Optional, List, Tuple
 
 from .. import disk
+from ..disk.device_model import BtrfsMountOption
 from ..hardware import SysInfo
 from ..menu import Menu
 from ..menu import TableMenu
@@ -214,6 +215,20 @@ def select_main_filesystem_format(advanced_options=False) -> disk.FilesystemType
 	return options[choice.single_value]
 
 
+def select_mount_options() -> List[str]:
+	prompt = str(_('Would you like to use compression or disable CoW?'))
+	options = [str(_('Use compression')), str(_('Disable Copy-on-Write'))]
+	choice = Menu(prompt, options, sort=False).run()
+
+	if choice.type_ == MenuSelectionType.Selection:
+		if choice.single_value == options[0]:
+			return [BtrfsMountOption.compress.value]
+		else:
+			return [BtrfsMountOption.nodatacow.value]
+
+	return []
+
+
 def suggest_single_disk_layout(
 	device: disk.BDevice,
 	filesystem_type: Optional[disk.FilesystemType] = None,
@@ -228,7 +243,7 @@ def suggest_single_disk_layout(
 	root_partition_size = disk.Size(20, disk.Unit.GiB, sector_size)
 	using_subvolumes = False
 	using_home_partition = False
-	compression = False
+	mount_options = []
 	device_size_gib = device.device_info.total_size
 
 	if filesystem_type == disk.FilesystemType.Btrfs:
@@ -236,9 +251,7 @@ def suggest_single_disk_layout(
 		choice = Menu(prompt, Menu.yes_no(), skip=False, default_option=Menu.yes()).run()
 		using_subvolumes = choice.value == Menu.yes()
 
-		prompt = str(_('Would you like to use BTRFS compression?'))
-		choice = Menu(prompt, Menu.yes_no(), skip=False, default_option=Menu.yes()).run()
-		compression = choice.value == Menu.yes()
+		mount_options = select_mount_options()
 
 	device_modification = disk.DeviceModification(device, wipe=True)
 
@@ -290,7 +303,7 @@ def suggest_single_disk_layout(
 		length=root_length,
 		mountpoint=Path('/') if not using_subvolumes else None,
 		fs_type=filesystem_type,
-		mount_options=['compress=zstd'] if compression else [],
+		mount_options=mount_options
 	)
 	device_modification.add_partition(root_partition)
 
@@ -323,7 +336,7 @@ def suggest_single_disk_layout(
 			length=home_length,
 			mountpoint=Path('/home'),
 			fs_type=filesystem_type,
-			mount_options=['compress=zstd'] if compression else []
+			mount_options=mount_options
 		)
 		device_modification.add_partition(home_partition)
 
@@ -344,7 +357,7 @@ def suggest_multi_disk_layout(
 	min_home_partition_size = disk.Size(40, disk.Unit.GiB, disk.SectorSize.default())
 	# rough estimate taking in to account user desktops etc. TODO: Catch user packages to detect size?
 	desired_root_partition_size = disk.Size(20, disk.Unit.GiB, disk.SectorSize.default())
-	compression = False
+	mount_options = []
 
 	if not filesystem_type:
 		filesystem_type = select_main_filesystem_format(advanced_options)
@@ -371,9 +384,7 @@ def suggest_multi_disk_layout(
 		return []
 
 	if filesystem_type == disk.FilesystemType.Btrfs:
-		prompt = str(_('Would you like to use BTRFS compression?'))
-		choice = Menu(prompt, Menu.yes_no(), skip=False, default_option=Menu.yes()).run()
-		compression = choice.value == Menu.yes()
+		mount_options = select_mount_options()
 
 	device_paths = ', '.join([str(d.device_info.path) for d in devices])
 
@@ -409,7 +420,7 @@ def suggest_multi_disk_layout(
 		start=root_start,
 		length=root_length,
 		mountpoint=Path('/'),
-		mount_options=['compress=zstd'] if compression else [],
+		mount_options=mount_options,
 		fs_type=filesystem_type
 	)
 	root_device_modification.add_partition(root_partition)
@@ -427,7 +438,7 @@ def suggest_multi_disk_layout(
 		start=home_start,
 		length=home_length,
 		mountpoint=Path('/home'),
-		mount_options=['compress=zstd'] if compression else [],
+		mount_options=mount_options,
 		fs_type=filesystem_type,
 	)
 	home_device_modification.add_partition(home_partition)