[pve-devel] [PATCH installer 0/7] tui: add ZFS/Btrfs RAID setup checks

[pve-devel] [PATCH installer 0/7] tui: add ZFS/Btrfs RAID setup checks

[Christoph Heiss]

This introduces the same checks when creating a RAID setup for ZFS and
Btrfs as in the GUI installer.

The main benefit is that the user now gets an error before the setup
dialog can even be closed, instead of during the install process.

Patch #1, #2 and #3 are some cleanups/fixes for things I noticed along
the way, #4 and #5 are prepatory patches. #6 then implements the actual
functionality.

Additionally, #7 contains unit tests for all the logic code paths that
were added, to ensure this cannot be accidentally be regressed.

pve-installer:

Christoph Heiss (7):
  gitignore: add cd-info.test
  tui: fix small typo in error message
  tui: simplify duplicate disk checking logic
  tui: deserialize boot type and disk blocksize from runtime env info
  tui: improve bootdisk dialog error handling
  tui: add RAID setup checks for ZFS/Btrfs
  tui: add tests for RAID setup checks

 .gitignore                                  |   1 +
 proxmox-tui-installer/src/options.rs        |   1 +
 proxmox-tui-installer/src/setup.rs          |  15 +-
 proxmox-tui-installer/src/views/bootdisk.rs | 369 ++++++++++++++++++--
 4 files changed, 347 insertions(+), 39 deletions(-)

--
2.41.0

[pve-devel] [PATCH installer 1/7] gitignore: add cd-info.test

[Christoph Heiss]

It's a testfile and should thus be ignored.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 .gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitignore b/.gitignore
index 8eb4e76..4212b7d 100644
--- a/.gitignore
+++ b/.gitignore
@@ -7,3 +7,4 @@
 /test*.img
 country.dat
 Cargo.lock
+/cd-info.test
-- 
2.41.0

[pve-devel] [PATCH installer 2/7] tui: fix small typo in error message

[Christoph Heiss]

Fixes: 994c4ff ("tui: add better error handling to BootdiskOptions::get_values()")
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 proxmox-tui-installer/src/views/bootdisk.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/proxmox-tui-installer/src/views/bootdisk.rs b/proxmox-tui-installer/src/views/bootdisk.rs
index 552cbc4..6ef814f 100644
--- a/proxmox-tui-installer/src/views/bootdisk.rs
+++ b/proxmox-tui-installer/src/views/bootdisk.rs
@@ -65,7 +65,7 @@ impl BootdiskOptionsView {
                 .and_then(|v| v.downcast_mut::<NamedView<FormView>>())
                 .map(NamedView::<FormView>::get_mut)
                 .and_then(|v| v.get_value::<SelectView<Disk>, _>(0))
-                .ok_or("failed to retrieve filesystem type")?;
+                .ok_or("failed to retrieve bootdisk")?;
 
             options.disks = vec![disk];
         }
-- 
2.41.0

[pve-devel] [PATCH installer 3/7] tui: simplify duplicate disk checking logic

[Christoph Heiss]

This reduces the logic from O(n^2) to O(n), which is always a good
thing.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 proxmox-tui-installer/src/views/bootdisk.rs | 39 ++++++++++++---------
 1 file changed, 23 insertions(+), 16 deletions(-)

diff --git a/proxmox-tui-installer/src/views/bootdisk.rs b/proxmox-tui-installer/src/views/bootdisk.rs
index 6ef814f..b0557a6 100644
--- a/proxmox-tui-installer/src/views/bootdisk.rs
+++ b/proxmox-tui-installer/src/views/bootdisk.rs
@@ -1,4 +1,4 @@
-use std::{cell::RefCell, marker::PhantomData, rc::Rc};
+use std::{cell::RefCell, collections::HashSet, marker::PhantomData, rc::Rc};
 
 use cursive::{
     view::{Nameable, Resizable, ViewWrapper},
@@ -536,21 +536,11 @@ fn advanced_options_view(disks: &[Disk], options: Rc<RefCell<BootdiskOptions>>)
                 })
                 .flatten();
 
-            if let Some(disks) = options.as_ref().map(|opts| &opts.disks) {
-                if disks.len() > 1 {
-                    for i in 0..(disks.len() - 1) {
-                        let check_disk = &disks[i];
-                        for disk in &disks[(i + 1)..] {
-                            if disk.index == check_disk.index {
-                                siv.add_layer(Dialog::info(format!(
-                                    "cannot select same disk ({}) twice",
-                                    disk.path
-                                )));
-                                return;
-                            }
-                        }
-                    }
-                }
+            if let Err(duplicate) = check_for_duplicate_disks(&options.disks) {
+                siv.add_layer(Dialog::info(format!(
+                    "Cannot select same disk twice: {duplicate}"
+                )));
+                return;
             }
 
             siv.pop_layer();
@@ -562,3 +552,20 @@ fn advanced_options_view(disks: &[Disk], options: Rc<RefCell<BootdiskOptions>>)
     .with_name("advanced-bootdisk-options-dialog")
     .max_size((120, 40))
 }
+
+/// Checks a list of disks for duplicate entries, using their index as key.
+///
+/// # Arguments
+///
+/// * `disks` - A list of disks to check for duplicates.
+fn check_for_duplicate_disks(disks: &[Disk]) -> Result<(), &Disk> {
+    let mut set = HashSet::new();
+
+    for disk in disks {
+        if !set.insert(&disk.index) {
+            return Err(disk);
+        }
+    }
+
+    Ok(())
+}
-- 
2.41.0

[pve-devel] [PATCH installer 4/7] tui: deserialize boot type and disk blocksize from runtime env info

[Christoph Heiss]

This is needed later on to check whether a RAID setup is compatible with
BIOS and 4Kn disks, in particular ZFS.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 proxmox-tui-installer/src/options.rs |  1 +
 proxmox-tui-installer/src/setup.rs   | 15 +++++++++++++--
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/proxmox-tui-installer/src/options.rs b/proxmox-tui-installer/src/options.rs
index c8e8215..dab1730 100644
--- a/proxmox-tui-installer/src/options.rs
+++ b/proxmox-tui-installer/src/options.rs
@@ -222,6 +222,7 @@ pub struct Disk {
     pub path: String,
     pub model: Option<String>,
     pub size: f64,
+    pub block_size: usize,
 }
 
 impl fmt::Display for Disk {
diff --git a/proxmox-tui-installer/src/setup.rs b/proxmox-tui-installer/src/setup.rs
index e8ee7f3..c56e608 100644
--- a/proxmox-tui-installer/src/setup.rs
+++ b/proxmox-tui-installer/src/setup.rs
@@ -262,13 +262,14 @@ fn deserialize_disks_map<'de, D>(deserializer: D) -> Result<Vec<Disk>, D::Error>
 where
     D: Deserializer<'de>,
 {
-    let disks = <Vec<(usize, String, f64, String, f64, String)>>::deserialize(deserializer)?;
+    let disks = <Vec<(usize, String, f64, String, usize, String)>>::deserialize(deserializer)?;
     Ok(disks
         .into_iter()
         .map(
             |(index, device, size_mb, model, logical_bsize, _syspath)| Disk {
                 index: index.to_string(),
-                size: (size_mb * logical_bsize) / 1024. / 1024. / 1024.,
+                size: (size_mb * logical_bsize as f64) / 1024. / 1024. / 1024.,
+                block_size: logical_bsize,
                 path: device,
                 model: (!model.is_empty()).then_some(model),
             },
@@ -351,6 +352,9 @@ where
 
 #[derive(Clone, Deserialize)]
 pub struct RuntimeInfo {
+    /// Whether is system was booted in (legacy) BIOS or UEFI mode.
+    pub boot_type: BootType,
+
     /// Detected country if available.
     pub country: Option<String>,
 
@@ -365,6 +369,13 @@ pub struct RuntimeInfo {
     pub total_memory: usize,
 }
 
+#[derive(Clone, Eq, Deserialize, PartialEq)]
+#[serde(rename_all = "lowercase")]
+pub enum BootType {
+    Bios,
+    Efi,
+}
+
 #[derive(Clone, Deserialize)]
 pub struct NetworkInfo {
     pub dns: Dns,
-- 
2.41.0

[pve-devel] [PATCH installer 5/7] tui: improve bootdisk dialog error handling

[Christoph Heiss]

Now we don't just fail silently, but instead give the user/developer
some indication what went wrong.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 proxmox-tui-installer/src/views/bootdisk.rs | 55 +++++++++++++++------
 1 file changed, 39 insertions(+), 16 deletions(-)

diff --git a/proxmox-tui-installer/src/views/bootdisk.rs b/proxmox-tui-installer/src/views/bootdisk.rs
index b0557a6..19a71cf 100644
--- a/proxmox-tui-installer/src/views/bootdisk.rs
+++ b/proxmox-tui-installer/src/views/bootdisk.rs
@@ -164,39 +164,52 @@ impl AdvancedBootdiskOptionsView {
         );
     }
 
-    fn get_values(&mut self) -> Option<BootdiskOptions> {
+    fn get_values(&mut self) -> Result<BootdiskOptions, String> {
         let fstype = self
             .view
-            .get_child(1)?
-            .downcast_ref::<FormView>()?
-            .get_value::<SelectView<FsType>, _>(0)?;
+            .get_child(1)
+            .and_then(|v| v.downcast_ref::<FormView>())
+            .and_then(|v| v.get_value::<SelectView<FsType>, _>(0))
+            .ok_or("Failed to retrieve filesystem type".to_owned())?;
 
-        let advanced = self.view.get_child_mut(3)?;
+        let advanced = self
+            .view
+            .get_child_mut(3)
+            .ok_or("Failed to retrieve advanced bootdisk options view".to_owned())?;
 
         if let Some(view) = advanced.downcast_mut::<LvmBootdiskOptionsView>() {
-            Some(BootdiskOptions {
+            let advanced = view
+                .get_values()
+                .map(AdvancedBootdiskOptions::Lvm)
+                .ok_or("Failed to retrieve advanced bootdisk options")?;
+
+            Ok(BootdiskOptions {
                 disks: vec![],
                 fstype,
-                advanced: view.get_values().map(AdvancedBootdiskOptions::Lvm)?,
+                advanced,
             })
         } else if let Some(view) = advanced.downcast_mut::<ZfsBootdiskOptionsView>() {
-            let (disks, advanced) = view.get_values()?;
+            let (disks, advanced) = view
+                .get_values()
+                .ok_or("Failed to retrieve advanced bootdisk options")?;
 
-            Some(BootdiskOptions {
+            Ok(BootdiskOptions {
                 disks,
                 fstype,
                 advanced: AdvancedBootdiskOptions::Zfs(advanced),
             })
         } else if let Some(view) = advanced.downcast_mut::<BtrfsBootdiskOptionsView>() {
-            let (disks, advanced) = view.get_values()?;
+            let (disks, advanced) = view
+                .get_values()
+                .ok_or("Failed to retrieve advanced bootdisk options")?;
 
-            Some(BootdiskOptions {
+            Ok(BootdiskOptions {
                 disks,
                 fstype,
                 advanced: AdvancedBootdiskOptions::Btrfs(advanced),
             })
         } else {
-            None
+            Err("Invalid bootdisk view state".to_owned())
         }
     }
 }
@@ -532,10 +545,22 @@ fn advanced_options_view(disks: &[Disk], options: Rc<RefCell<BootdiskOptions>>)
                 .call_on_name("advanced-bootdisk-options-dialog", |view: &mut Dialog| {
                     view.get_content_mut()
                         .downcast_mut()
-                        .and_then(AdvancedBootdiskOptionsView::get_values)
+                        .map(AdvancedBootdiskOptionsView::get_values)
                 })
                 .flatten();
 
+            let options = match options {
+                Some(Ok(options)) => options,
+                Some(Err(err)) => {
+                    siv.add_layer(Dialog::info(err));
+                    return;
+                }
+                None => {
+                    siv.add_layer(Dialog::info("Failed to retrieve bootdisk options view"));
+                    return;
+                }
+            };
+
             if let Err(duplicate) = check_for_duplicate_disks(&options.disks) {
                 siv.add_layer(Dialog::info(format!(
                     "Cannot select same disk twice: {duplicate}"
@@ -544,9 +569,7 @@ fn advanced_options_view(disks: &[Disk], options: Rc<RefCell<BootdiskOptions>>)
             }
 
             siv.pop_layer();
-            if let Some(options) = options {
-                *(*options_ref).borrow_mut() = options;
-            }
+            *(*options_ref).borrow_mut() = options;
         }
     })
     .with_name("advanced-bootdisk-options-dialog")
-- 
2.41.0

[pve-devel] [PATCH installer 6/7] tui: add RAID setup checks for ZFS/Btrfs

[Christoph Heiss]

This adds early checks when setting up ZFS and Btrfs RAIDs, such as
minimum number of disks in the RAID, mirror sizes and legacy BIOS
compatibility.

The same rules as the GUI uses are applied, which unfortunaly means that
this logic is essentially duplicated between the GUI and TUI.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 proxmox-tui-installer/src/views/bootdisk.rs | 136 +++++++++++++++++++-
 1 file changed, 129 insertions(+), 7 deletions(-)

diff --git a/proxmox-tui-installer/src/views/bootdisk.rs b/proxmox-tui-installer/src/views/bootdisk.rs
index 19a71cf..9056323 100644
--- a/proxmox-tui-installer/src/views/bootdisk.rs
+++ b/proxmox-tui-installer/src/views/bootdisk.rs
@@ -10,11 +10,15 @@ use cursive::{
 };
 
 use super::{DiskSizeEditView, FormView, IntegerEditView};
-use crate::options::{
-    AdvancedBootdiskOptions, BootdiskOptions, BtrfsBootdiskOptions, Disk, FsType,
-    LvmBootdiskOptions, ZfsBootdiskOptions, FS_TYPES, ZFS_CHECKSUM_OPTIONS, ZFS_COMPRESS_OPTIONS,
+use crate::{
+    options::{
+        AdvancedBootdiskOptions, BootdiskOptions, BtrfsBootdiskOptions, BtrfsRaidLevel, Disk,
+        FsType, LvmBootdiskOptions, ZfsBootdiskOptions, ZfsRaidLevel, FS_TYPES,
+        ZFS_CHECKSUM_OPTIONS, ZFS_COMPRESS_OPTIONS,
+    },
+    setup::{BootType, RuntimeInfo},
 };
-use crate::setup::ProxmoxProduct;
+use crate::{setup::ProxmoxProduct, InstallerState};
 
 pub struct BootdiskOptionsView {
     view: LinearLayout,
@@ -164,7 +168,7 @@ impl AdvancedBootdiskOptionsView {
         );
     }
 
-    fn get_values(&mut self) -> Result<BootdiskOptions, String> {
+    fn get_values(&mut self, runinfo: &RuntimeInfo) -> Result<BootdiskOptions, String> {
         let fstype = self
             .view
             .get_child(1)
@@ -193,6 +197,11 @@ impl AdvancedBootdiskOptionsView {
                 .get_values()
                 .ok_or("Failed to retrieve advanced bootdisk options")?;
 
+            if let FsType::Zfs(level) = fstype {
+                check_zfs_raid_config(runinfo, level, &disks)
+                    .map_err(|err| format!("{fstype}: {err}"))?;
+            }
+
             Ok(BootdiskOptions {
                 disks,
                 fstype,
@@ -203,6 +212,10 @@ impl AdvancedBootdiskOptionsView {
                 .get_values()
                 .ok_or("Failed to retrieve advanced bootdisk options")?;
 
+            if let FsType::Btrfs(level) = fstype {
+                check_btrfs_raid_config(level, &disks).map_err(|err| format!("{fstype}: {err}"))?;
+            }
+
             Ok(BootdiskOptions {
                 disks,
                 fstype,
@@ -541,11 +554,17 @@ fn advanced_options_view(disks: &[Disk], options: Rc<RefCell<BootdiskOptions>>)
     .button("Ok", {
         let options_ref = options.clone();
         move |siv| {
+            let runinfo = siv
+                .user_data::<InstallerState>()
+                .unwrap()
+                .runtime_info
+                .clone();
+
             let options = siv
                 .call_on_name("advanced-bootdisk-options-dialog", |view: &mut Dialog| {
                     view.get_content_mut()
-                        .downcast_mut()
-                        .map(AdvancedBootdiskOptionsView::get_values)
+                        .downcast_mut::<AdvancedBootdiskOptionsView>()
+                        .map(|v| v.get_values(&runinfo))
                 })
                 .flatten();
 
@@ -592,3 +611,106 @@ fn check_for_duplicate_disks(disks: &[Disk]) -> Result<(), &Disk> {
 
     Ok(())
 }
+
+/// Simple wrapper which returns an descriptive error if the list of disks is too short.
+///
+/// # Arguments
+///
+/// * `disks` - A list of disks to check the lenght of.
+/// * `min` - Minimum number of disks
+fn check_raid_min_disks(disks: &[Disk], min: usize) -> Result<(), String> {
+    if disks.len() < min {
+        Err(format!("Need at least {min} disks"))
+    } else {
+        Ok(())
+    }
+}
+
+/// Checks whether a user-supplied ZFS RAID setup is valid or not, such as disk sizes, minimum
+/// number of disks and legacy BIOS compatibility.
+///
+/// # Arguments
+///
+/// * `runinfo` - `RuntimeInfo` instance of currently running system
+/// * `level` - The targeted ZFS RAID level by the user.
+/// * `disks` - List of disks designated as RAID targets.
+fn check_zfs_raid_config(
+    runinfo: &RuntimeInfo,
+    level: ZfsRaidLevel,
+    disks: &[Disk],
+) -> Result<(), String> {
+    // See also Proxmox/Install.pm:get_zfs_raid_setup()
+
+    for disk in disks {
+        if runinfo.boot_type != BootType::Efi && disk.block_size == 4096 {
+            return Err("Booting from 4Kn drive in legacy BIOS mode is not supported.".to_owned());
+        }
+    }
+
+    let check_mirror_size = |disk1: &Disk, disk2: &Disk| {
+        if (disk1.size - disk2.size).abs() > disk1.size / 10. {
+            Err(format!(
+                "Mirrored disks must have same size:\n\n  * {disk1}\n  * {disk2}"
+            ))
+        } else {
+            Ok(())
+        }
+    };
+
+    match level {
+        ZfsRaidLevel::Raid0 => check_raid_min_disks(disks, 1)?,
+        ZfsRaidLevel::Raid1 => {
+            check_raid_min_disks(disks, 2)?;
+            for disk in disks {
+                check_mirror_size(&disks[0], disk)?;
+            }
+        }
+        ZfsRaidLevel::Raid10 => {
+            check_raid_min_disks(disks, 4)?;
+            // Pairs need to have the same size
+            for i in (0..disks.len()).step_by(2) {
+                check_mirror_size(&disks[i], &disks[i + 1])?;
+            }
+        }
+        // For RAID-Z: minimum disks number is level + 2
+        ZfsRaidLevel::RaidZ => {
+            check_raid_min_disks(disks, 3)?;
+            for disk in disks {
+                check_mirror_size(&disks[0], disk)?;
+            }
+        }
+        ZfsRaidLevel::RaidZ2 => {
+            check_raid_min_disks(disks, 4)?;
+            for disk in disks {
+                check_mirror_size(&disks[0], disk)?;
+            }
+        }
+        ZfsRaidLevel::RaidZ3 => {
+            check_raid_min_disks(disks, 5)?;
+            for disk in disks {
+                check_mirror_size(&disks[0], disk)?;
+            }
+        }
+    }
+
+    Ok(())
+}
+
+/// Checks whether a user-supplied Btrfs RAID setup is valid or not, such as minimum
+/// number of disks.
+///
+/// # Arguments
+///
+/// * `level` - The targeted Btrfs RAID level by the user.
+/// * `disks` - List of disks designated as RAID targets.
+fn check_btrfs_raid_config(level: BtrfsRaidLevel, disks: &[Disk]) -> Result<(), String> {
+    // See also Proxmox/Install.pm:get_btrfs_raid_setup()
+
+    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)?,
+    }
+
+    Ok(())
+}
-- 
2.41.0

[pve-devel] [PATCH installer 7/7] tui: add tests for RAID setup checks

[Christoph Heiss]

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 proxmox-tui-installer/src/views/bootdisk.rs | 143 ++++++++++++++++++++
 1 file changed, 143 insertions(+)

diff --git a/proxmox-tui-installer/src/views/bootdisk.rs b/proxmox-tui-installer/src/views/bootdisk.rs
index 9056323..8303cd7 100644
--- a/proxmox-tui-installer/src/views/bootdisk.rs
+++ b/proxmox-tui-installer/src/views/bootdisk.rs
@@ -714,3 +714,146 @@ fn check_btrfs_raid_config(level: BtrfsRaidLevel, disks: &[Disk]) -> Result<(),
 
     Ok(())
 }
+
+#[cfg(test)]
+mod tests {
+    use std::collections::HashMap;
+
+    use super::*;
+    use crate::setup::{Dns, NetworkInfo};
+
+    fn dummy_disk(index: usize) -> Disk {
+        Disk {
+            index: index.to_string(),
+            path: format!("/dev/dummy{index}"),
+            model: Some("Dummy disk".to_owned()),
+            size: 1024. * 1024. * 1024. * 8.,
+            block_size: 512,
+        }
+    }
+
+    fn dummy_disks(num: usize) -> Vec<Disk> {
+        (0..num).map(dummy_disk).collect()
+    }
+
+    fn dummy_runinfo(boot_type: BootType) -> RuntimeInfo {
+        RuntimeInfo {
+            boot_type,
+            country: Some("at".to_owned()),
+            disks: dummy_disks(4),
+            network: NetworkInfo {
+                dns: Dns {
+                    domain: None,
+                    dns: vec![],
+                },
+                routes: None,
+                interfaces: HashMap::new(),
+            },
+            total_memory: 1024 * 1024 * 1024 * 64,
+        }
+    }
+
+    #[test]
+    fn duplicate_disks() {
+        assert!(check_for_duplicate_disks(&dummy_disks(2)).is_ok());
+        assert_eq!(
+            check_for_duplicate_disks(&[
+                dummy_disk(0),
+                dummy_disk(1),
+                dummy_disk(2),
+                dummy_disk(2),
+                dummy_disk(3),
+            ]),
+            Err(&dummy_disk(2)),
+        );
+    }
+
+    #[test]
+    fn raid_min_disks() {
+        let disks = dummy_disks(10);
+
+        assert!(check_raid_min_disks(&disks[..1], 2).is_err());
+        assert!(check_raid_min_disks(&disks[..1], 1).is_ok());
+        assert!(check_raid_min_disks(&disks, 1).is_ok());
+    }
+
+    #[test]
+    fn btrfs_raid() {
+        let disks = dummy_disks(10);
+
+        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid0, &[]).is_err());
+        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid0, &disks[..1]).is_ok());
+        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid0, &disks).is_ok());
+
+        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid1, &[]).is_err());
+        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid1, &disks[..1]).is_err());
+        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid1, &disks[..2]).is_ok());
+        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).is_ok());
+    }
+
+    #[test]
+    fn zfs_raid_bios() {
+        let disks = dummy_disks(10);
+        let runinfo = dummy_runinfo(BootType::Bios);
+
+        assert!(check_zfs_raid_config(&runinfo, ZfsRaidLevel::Raid0, &[]).is_err());
+        assert!(check_zfs_raid_config(&runinfo, ZfsRaidLevel::Raid0, &disks[..1]).is_ok());
+        assert!(check_zfs_raid_config(&runinfo, ZfsRaidLevel::Raid0, &disks).is_ok());
+
+        assert!(check_zfs_raid_config(&runinfo, ZfsRaidLevel::Raid1, &[]).is_err());
+        assert!(check_zfs_raid_config(&runinfo, ZfsRaidLevel::Raid1, &disks[..2]).is_ok());
+        assert!(check_zfs_raid_config(&runinfo, ZfsRaidLevel::Raid1, &disks).is_ok());
+
+        assert!(check_zfs_raid_config(&runinfo, ZfsRaidLevel::Raid10, &[]).is_err());
+        assert!(check_zfs_raid_config(&runinfo, ZfsRaidLevel::Raid10, &dummy_disks(4)).is_ok());
+        assert!(check_zfs_raid_config(&runinfo, ZfsRaidLevel::Raid10, &disks).is_ok());
+
+        assert!(check_zfs_raid_config(&runinfo, ZfsRaidLevel::RaidZ, &[]).is_err());
+        assert!(check_zfs_raid_config(&runinfo, ZfsRaidLevel::RaidZ, &disks[..2]).is_err());
+        assert!(check_zfs_raid_config(&runinfo, ZfsRaidLevel::RaidZ, &disks[..3]).is_ok());
+        assert!(check_zfs_raid_config(&runinfo, ZfsRaidLevel::RaidZ, &disks).is_ok());
+
+        assert!(check_zfs_raid_config(&runinfo, ZfsRaidLevel::RaidZ2, &[]).is_err());
+        assert!(check_zfs_raid_config(&runinfo, ZfsRaidLevel::RaidZ2, &disks[..3]).is_err());
+        assert!(check_zfs_raid_config(&runinfo, ZfsRaidLevel::RaidZ2, &disks[..4]).is_ok());
+        assert!(check_zfs_raid_config(&runinfo, ZfsRaidLevel::RaidZ2, &disks).is_ok());
+
+        assert!(check_zfs_raid_config(&runinfo, ZfsRaidLevel::RaidZ3, &[]).is_err());
+        assert!(check_zfs_raid_config(&runinfo, ZfsRaidLevel::RaidZ3, &disks[..4]).is_err());
+        assert!(check_zfs_raid_config(&runinfo, ZfsRaidLevel::RaidZ3, &disks[..5]).is_ok());
+        assert!(check_zfs_raid_config(&runinfo, ZfsRaidLevel::RaidZ3, &disks).is_ok());
+
+        for i in 0..10 {
+            let mut disks = dummy_disks(10);
+            disks[i].block_size = 4096;
+
+            // Must fail if /any/ of the disks are 4Kn
+            assert!(check_zfs_raid_config(&runinfo, ZfsRaidLevel::Raid0, &disks).is_err());
+            assert!(check_zfs_raid_config(&runinfo, ZfsRaidLevel::Raid1, &disks).is_err());
+            assert!(check_zfs_raid_config(&runinfo, ZfsRaidLevel::Raid10, &disks).is_err());
+            assert!(check_zfs_raid_config(&runinfo, ZfsRaidLevel::RaidZ, &disks).is_err());
+            assert!(check_zfs_raid_config(&runinfo, ZfsRaidLevel::RaidZ2, &disks).is_err());
+            assert!(check_zfs_raid_config(&runinfo, ZfsRaidLevel::RaidZ3, &disks).is_err());
+        }
+    }
+
+    #[test]
+    fn zfs_raid_efi() {
+        let disks = dummy_disks(10);
+        let runinfo = dummy_runinfo(BootType::Efi);
+
+        assert!(check_zfs_raid_config(&runinfo, ZfsRaidLevel::Raid0, &disks[..1]).is_ok());
+        assert!(check_zfs_raid_config(&runinfo, ZfsRaidLevel::Raid0, &disks).is_ok());
+
+        assert!(check_zfs_raid_config(&runinfo, ZfsRaidLevel::Raid1, &disks[..2]).is_ok());
+        assert!(check_zfs_raid_config(&runinfo, ZfsRaidLevel::Raid1, &disks).is_ok());
+
+        assert!(check_zfs_raid_config(&runinfo, ZfsRaidLevel::Raid10, &dummy_disks(4)).is_ok());
+        assert!(check_zfs_raid_config(&runinfo, ZfsRaidLevel::Raid10, &disks).is_ok());
+    }
+}
-- 
2.41.0

[pve-devel] partially-applied: [PATCH installer 0/7] tui: add ZFS/Btrfs RAID setup checks

[Thomas Lamprecht]

On 13/07/2023 11:49, Christoph Heiss wrote:
> This introduces the same checks when creating a RAID setup for ZFS and
> Btrfs as in the GUI installer.
> 
> The main benefit is that the user now gets an error before the setup
> dialog can even be closed, instead of during the install process.
> 
> Patch #1, #2 and #3 are some cleanups/fixes for things I noticed along
> the way, #4 and #5 are prepatory patches. #6 then implements the actual
> functionality.
> 
> Additionally, #7 contains unit tests for all the logic code paths that
> were added, to ensure this cannot be accidentally be regressed.
> 
> pve-installer:
> 
> Christoph Heiss (7):
>   gitignore: add cd-info.test
>   tui: fix small typo in error message

applied above two, but below cause build/test failures or do not apply
(anymore).

>   tui: simplify duplicate disk checking logic
>   tui: deserialize boot type and disk blocksize from runtime env info
>   tui: improve bootdisk dialog error handling
>   tui: add RAID setup checks for ZFS/Btrfs
>   tui: add tests for RAID setup checks