public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH installer v2 0/6] tui: add ZFS/Btrfs RAID setup checks
@ 2023-07-24 10:14 Christoph Heiss
  2023-07-24 10:14 ` [pve-devel] [PATCH installer v2 1/6] tui: simplify duplicate disk checking logic Christoph Heiss
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Christoph Heiss @ 2023-07-24 10:14 UTC (permalink / raw)
  To: pve-devel

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 is just some cleanup I noticed along the way, #2 and #3 are
preparatory patches. #4 then implements the actual functionality and #5
unit tests for all that. #6 just enables dh_auto_test in the end.

Notable changes v1 -> v2:
  * Rebased series on latest master
  * Added patch #6, enabling dh_auto_test

v1: https://lists.proxmox.com/pipermail/pve-devel/2023-July/058077.html

pve-installer:

Christoph Heiss (6):
  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
  d/rules: enable dh_auto_test

 Makefile                                    |   3 +
 debian/rules                                |   3 -
 proxmox-tui-installer/src/options.rs        |   1 +
 proxmox-tui-installer/src/setup.rs          |  15 +-
 proxmox-tui-installer/src/views/bootdisk.rs | 368 ++++++++++++++++++--
 5 files changed, 349 insertions(+), 41 deletions(-)

--
2.41.0





^ permalink raw reply	[flat|nested] 8+ messages in thread

* [pve-devel] [PATCH installer v2 1/6] tui: simplify duplicate disk checking logic
  2023-07-24 10:14 [pve-devel] [PATCH installer v2 0/6] tui: add ZFS/Btrfs RAID setup checks Christoph Heiss
@ 2023-07-24 10:14 ` Christoph Heiss
  2023-07-24 10:14 ` [pve-devel] [PATCH installer v2 2/6] tui: deserialize boot type and disk blocksize from runtime env info Christoph Heiss
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Christoph Heiss @ 2023-07-24 10:14 UTC (permalink / raw)
  To: pve-devel

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>
---
Changes v1 -> v2:
  * No changes

 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





^ permalink raw reply	[flat|nested] 8+ messages in thread

* [pve-devel] [PATCH installer v2 2/6] tui: deserialize boot type and disk blocksize from runtime env info
  2023-07-24 10:14 [pve-devel] [PATCH installer v2 0/6] tui: add ZFS/Btrfs RAID setup checks Christoph Heiss
  2023-07-24 10:14 ` [pve-devel] [PATCH installer v2 1/6] tui: simplify duplicate disk checking logic Christoph Heiss
@ 2023-07-24 10:14 ` Christoph Heiss
  2023-07-24 10:14 ` [pve-devel] [PATCH installer v2 3/6] tui: improve bootdisk dialog error handling Christoph Heiss
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Christoph Heiss @ 2023-07-24 10:14 UTC (permalink / raw)
  To: pve-devel

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>
---
Changes v1 -> v2:
  * No changes

 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 5e07f36..e51bb4d 100644
--- a/proxmox-tui-installer/src/setup.rs
+++ b/proxmox-tui-installer/src/setup.rs
@@ -277,13 +277,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),
             },
@@ -366,6 +367,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>,

@@ -384,6 +388,13 @@ pub struct RuntimeInfo {
     pub hvm_supported: bool,
 }

+#[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





^ permalink raw reply	[flat|nested] 8+ messages in thread

* [pve-devel] [PATCH installer v2 3/6] tui: improve bootdisk dialog error handling
  2023-07-24 10:14 [pve-devel] [PATCH installer v2 0/6] tui: add ZFS/Btrfs RAID setup checks Christoph Heiss
  2023-07-24 10:14 ` [pve-devel] [PATCH installer v2 1/6] tui: simplify duplicate disk checking logic Christoph Heiss
  2023-07-24 10:14 ` [pve-devel] [PATCH installer v2 2/6] tui: deserialize boot type and disk blocksize from runtime env info Christoph Heiss
@ 2023-07-24 10:14 ` Christoph Heiss
  2023-07-24 10:14 ` [pve-devel] [PATCH installer v2 4/6] tui: add RAID setup checks for ZFS/Btrfs Christoph Heiss
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Christoph Heiss @ 2023-07-24 10:14 UTC (permalink / raw)
  To: pve-devel

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>
---
Changes v1 -> v2:
  * No changes

 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





^ permalink raw reply	[flat|nested] 8+ messages in thread

* [pve-devel] [PATCH installer v2 4/6] tui: add RAID setup checks for ZFS/Btrfs
  2023-07-24 10:14 [pve-devel] [PATCH installer v2 0/6] tui: add ZFS/Btrfs RAID setup checks Christoph Heiss
                   ` (2 preceding siblings ...)
  2023-07-24 10:14 ` [pve-devel] [PATCH installer v2 3/6] tui: improve bootdisk dialog error handling Christoph Heiss
@ 2023-07-24 10:14 ` Christoph Heiss
  2023-07-24 10:14 ` [pve-devel] [PATCH installer v2 5/6] tui: add tests for RAID setup checks Christoph Heiss
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Christoph Heiss @ 2023-07-24 10:14 UTC (permalink / raw)
  To: pve-devel

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>
---
Changes v1 -> v2:
  * No changes

 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





^ permalink raw reply	[flat|nested] 8+ messages in thread

* [pve-devel] [PATCH installer v2 5/6] tui: add tests for RAID setup checks
  2023-07-24 10:14 [pve-devel] [PATCH installer v2 0/6] tui: add ZFS/Btrfs RAID setup checks Christoph Heiss
                   ` (3 preceding siblings ...)
  2023-07-24 10:14 ` [pve-devel] [PATCH installer v2 4/6] tui: add RAID setup checks for ZFS/Btrfs Christoph Heiss
@ 2023-07-24 10:14 ` Christoph Heiss
  2023-07-24 10:14 ` [pve-devel] [PATCH installer v2 6/6] d/rules: enable dh_auto_test Christoph Heiss
  2023-07-25  9:38 ` [pve-devel] applied-series: [PATCH installer v2 0/6] tui: add ZFS/Btrfs RAID setup checks Thomas Lamprecht
  6 siblings, 0 replies; 8+ messages in thread
From: Christoph Heiss @ 2023-07-24 10:14 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
  * Added more ZFS EFI tests by splitting out common tests for ZFS

 proxmox-tui-installer/src/views/bootdisk.rs | 141 ++++++++++++++++++++
 1 file changed, 141 insertions(+)

diff --git a/proxmox-tui-installer/src/views/bootdisk.rs b/proxmox-tui-installer/src/views/bootdisk.rs
index 9056323..a018e71 100644
--- a/proxmox-tui-installer/src/views/bootdisk.rs
+++ b/proxmox-tui-installer/src/views/bootdisk.rs
@@ -714,3 +714,144 @@ 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,
+            hvm_supported: true,
+        }
+    }
+
+    #[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);
+
+        zfs_common_tests(&disks, &runinfo);
+
+        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);
+
+        zfs_common_tests(&disks, &runinfo);
+    }
+
+    fn zfs_common_tests(disks: &[Disk], runinfo: &RuntimeInfo) {
+        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());
+    }
+}
--
2.41.0





^ permalink raw reply	[flat|nested] 8+ messages in thread

* [pve-devel] [PATCH installer v2 6/6] d/rules: enable dh_auto_test
  2023-07-24 10:14 [pve-devel] [PATCH installer v2 0/6] tui: add ZFS/Btrfs RAID setup checks Christoph Heiss
                   ` (4 preceding siblings ...)
  2023-07-24 10:14 ` [pve-devel] [PATCH installer v2 5/6] tui: add tests for RAID setup checks Christoph Heiss
@ 2023-07-24 10:14 ` Christoph Heiss
  2023-07-25  9:38 ` [pve-devel] applied-series: [PATCH installer v2 0/6] tui: add ZFS/Btrfs RAID setup checks Thomas Lamprecht
  6 siblings, 0 replies; 8+ messages in thread
From: Christoph Heiss @ 2023-07-24 10:14 UTC (permalink / raw)
  To: pve-devel

Since we now have a test suite (or, at least the TUI installer in this
case), we can enable dh_auto_test so that it will be run on package
builds.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
  * New patch in v2

 Makefile     | 3 +++
 debian/rules | 3 ---
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 18fdea3..dc180b2 100644
--- a/Makefile
+++ b/Makefile
@@ -75,6 +75,9 @@ $(DSC): $(BUILDDIR)
 sbuild: $(DSC)
 	sbuild $(DSC)

+test:
+	$(CARGO) test --workspace $(CARGO_BUILD_ARGS)
+
 DESTDIR=
 VARLIBDIR=$(DESTDIR)/var/lib/proxmox-installer
 HTMLDIR=$(VARLIBDIR)/html/common
diff --git a/debian/rules b/debian/rules
index 6472b32..1c03065 100755
--- a/debian/rules
+++ b/debian/rules
@@ -10,6 +10,3 @@ export BUILD_MODE=release

 override_dh_missing:
 	dh_missing --fail-missing
-
-override_dh_auto_test:
-	# do nothing here
--
2.41.0





^ permalink raw reply	[flat|nested] 8+ messages in thread

* [pve-devel] applied-series: [PATCH installer v2 0/6] tui: add ZFS/Btrfs RAID setup checks
  2023-07-24 10:14 [pve-devel] [PATCH installer v2 0/6] tui: add ZFS/Btrfs RAID setup checks Christoph Heiss
                   ` (5 preceding siblings ...)
  2023-07-24 10:14 ` [pve-devel] [PATCH installer v2 6/6] d/rules: enable dh_auto_test Christoph Heiss
@ 2023-07-25  9:38 ` Thomas Lamprecht
  6 siblings, 0 replies; 8+ messages in thread
From: Thomas Lamprecht @ 2023-07-25  9:38 UTC (permalink / raw)
  To: Proxmox VE development discussion, Christoph Heiss

On 24/07/2023 12:14, Christoph Heiss wrote:
> pve-installer:
> 
> Christoph Heiss (6):
>   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
>   d/rules: enable dh_auto_test
> 

applied series, thanks!




^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-07-25  9:39 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-24 10:14 [pve-devel] [PATCH installer v2 0/6] tui: add ZFS/Btrfs RAID setup checks Christoph Heiss
2023-07-24 10:14 ` [pve-devel] [PATCH installer v2 1/6] tui: simplify duplicate disk checking logic Christoph Heiss
2023-07-24 10:14 ` [pve-devel] [PATCH installer v2 2/6] tui: deserialize boot type and disk blocksize from runtime env info Christoph Heiss
2023-07-24 10:14 ` [pve-devel] [PATCH installer v2 3/6] tui: improve bootdisk dialog error handling Christoph Heiss
2023-07-24 10:14 ` [pve-devel] [PATCH installer v2 4/6] tui: add RAID setup checks for ZFS/Btrfs Christoph Heiss
2023-07-24 10:14 ` [pve-devel] [PATCH installer v2 5/6] tui: add tests for RAID setup checks Christoph Heiss
2023-07-24 10:14 ` [pve-devel] [PATCH installer v2 6/6] d/rules: enable dh_auto_test Christoph Heiss
2023-07-25  9:38 ` [pve-devel] applied-series: [PATCH installer v2 0/6] tui: add ZFS/Btrfs RAID setup checks Thomas Lamprecht

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal