[pve-devel] [RFC PATCH 1/2] install: btrfs: fix raid level falling back to single mode

[pve-devel] [RFC PATCH 1/2] install: btrfs: fix raid level falling back to single mode

[Daniel Kral]

From a user's perspective, the BTRFS single mode has been removed since
d85180e ("tui: rename raid levels 0/1 to align with GUI installer"). The
user can now only select at least the "RAID0" level, but if a user
selects any raid level with only one disk configured on the system,
BTRFS will be setup in 'single' mode instead of the chosen raid level.

The TUI installer has a separate check for this, but the GUI installer
as well as the auto installer will silently fallback to single mode,
which could be confusing for and unwanted by the user.

Therefore, remove the BTRFS single mode from being selected when
configuring disks in the GUI installer and during the installation in
general, which makes the auto installer fail if the wrong amount of
disks are selected for the specified raid level. This makes btrfs' raid
disk count validation align with the one from zfs.

Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---

Discussion

BTRFS didn't allow a single-disk RAID0 configuration before kernel 5.15
and AFAIK also silently used the single profile for that case, but this
has changed since then.

If we want users to still be able to create a BTRFS filesystem in single
mode (which seems very reasonable), I can do a v2 or followup to add a
"btrfs (single)" entry to the disk setup.

Testing

I have tested this by the usual installer test procedure (debug shell,
patch files, launch target installer manually) and the GUI installer
disallows creating a btrfs raid1 or raid10 with only one disk, but
allows it with at least the correct amount of disks. The same applies to
the autoinstaller with the same test cases.

Unpatched a single-disk RAID0 install resulted in:

```
root@pve:~# btrfs filesystem usage -T .
[ ... ]
             Data   Metadata  System
Id Path      single single    single   Unallocated Total    Slack
-- --------- ------ --------- -------- ----------- -------- -------
 1 /dev/sda3 4.01GB 520.00MiB  4.00MiB    26.98GiB 31.50GiB 3.50KiB
[ ... ]
```

and patched a single-disk RAID0 install results in:

```
```
root@pve:~# btrfs filesystem usage -T .
[ ... ]
             Data   Metadata  System
Id Path      RAID0  RAID0     RAID0    Unallocated Total    Slack
-- --------- ------ --------- -------- ----------- -------- -------
 1 /dev/sda3 4.00GB 512.00MiB  8.00MiB    26.99GiB 31.50GiB 3.50KiB
[ ... ]
```
```

 Proxmox/Install.pm | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/Proxmox/Install.pm b/Proxmox/Install.pm
index c0a17b2..b72a83e 100644
--- a/Proxmox/Install.pm
+++ b/Proxmox/Install.pm
@@ -359,20 +359,16 @@ sub get_btrfs_raid_setup {
 
     my $mode;
 
-    if ($diskcount == 1) {
-	$mode = 'single';
+    if ($filesys eq 'btrfs (RAID0)') {
+	$mode = 'raid0';
+    } elsif ($filesys eq 'btrfs (RAID1)') {
+	die "btrfs (RAID1) needs at least 2 devices\n" if $diskcount < 2;
+	$mode = 'raid1';
+    } elsif ($filesys eq 'btrfs (RAID10)') {
+	die "btrfs (RAID10) needs at least 4 devices\n" if $diskcount < 4;
+	$mode = 'raid10';
     } else {
-	if ($filesys eq 'btrfs (RAID0)') {
-	    $mode = 'raid0';
-	} elsif ($filesys eq 'btrfs (RAID1)') {
-	    die "btrfs (RAID1) needs at least 2 devices\n" if $diskcount < 2;
-	    $mode = 'raid1';
-	} elsif ($filesys eq 'btrfs (RAID10)') {
-	    die "btrfs (RAID10) needs at least 4 devices\n" if $diskcount < 4;
-	    $mode = 'raid10';
-	} else {
-	    die "unknown btrfs mode '$filesys'\n";
-	}
+	die "unknown btrfs mode '$filesys'\n";
     }
 
     return ($devlist, $mode);
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [RFC PATCH 2/2] common: btrfs: lower minimum amount of disks for raid10 to 2

[Daniel Kral]

As the installer allows single-disk RAID0 configurations and BTRFS
allows to create a filesystem with the RAID10 profile with only two
disks since kernel version 5.15 [0], lower the minimum amount of disks
the installer requires for a BTRFS RAID10 setup.

The motiviation for this is to allow users to create a BTRFS RAID10
configuration even though they do not have the necessary disks ready at
setup time itself without needing to convert the profile afterwards.

[0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b2f78e88052bc0bee56bbf646d245fcfb431a873

Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
Discussion

If this patch seems like something worthwile, I think it would be
necessary to have some sort of warning popup for 2 <= $diskcount < 4 in
RAID10, and maybe also the same for $diskcount == 1 in RAID0, that
there's no advantage to create a degenerate RAID0/10 without planning to
add at least 1/2 disks later. I would add this in a v2 or followup if
this gets ACKed.

 Proxmox/Install.pm                          | 2 +-
 proxmox-installer-common/src/disk_checks.rs | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Proxmox/Install.pm b/Proxmox/Install.pm
index b72a83e..d52d17b 100644
--- a/Proxmox/Install.pm
+++ b/Proxmox/Install.pm
@@ -365,7 +365,7 @@ sub get_btrfs_raid_setup {
 	die "btrfs (RAID1) needs at least 2 devices\n" if $diskcount < 2;
 	$mode = 'raid1';
     } elsif ($filesys eq 'btrfs (RAID10)') {
-	die "btrfs (RAID10) needs at least 4 devices\n" if $diskcount < 4;
+	die "btrfs (RAID10) needs at least 2 devices\n" if $diskcount < 2;
 	$mode = 'raid10';
     } else {
 	die "unknown btrfs mode '$filesys'\n";
diff --git a/proxmox-installer-common/src/disk_checks.rs b/proxmox-installer-common/src/disk_checks.rs
index ecc43bd..bd1c54c 100644
--- a/proxmox-installer-common/src/disk_checks.rs
+++ b/proxmox-installer-common/src/disk_checks.rs
@@ -129,7 +129,7 @@ pub fn check_btrfs_raid_config(level: BtrfsRaidLevel, disks: &[Disk]) -> Result<
     match level {
         BtrfsRaidLevel::Raid0 => check_raid_min_disks(disks, 1)?,
         BtrfsRaidLevel::Raid1 => check_raid_min_disks(disks, 2)?,
-        BtrfsRaidLevel::Raid10 => check_raid_min_disks(disks, 4)?,
+        BtrfsRaidLevel::Raid10 => check_raid_min_disks(disks, 2)?,
     }
 
     Ok(())
@@ -204,8 +204,8 @@ mod tests {
         assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid1, &disks).is_ok());
 
         assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid10, &[]).is_err());
-        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid10, &disks[..3]).is_err());
-        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid10, &disks[..4]).is_ok());
+        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid10, &disks[..1]).is_err());
+        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid10, &disks[..2]).is_ok());
         assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid10, &disks).is_ok());
     }
 
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel