public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Christoph Heiss <c.heiss@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH installer v2 4/6] tui: add RAID setup checks for ZFS/Btrfs
Date: Mon, 24 Jul 2023 12:14:48 +0200	[thread overview]
Message-ID: <20230724101517.462214-5-c.heiss@proxmox.com> (raw)
In-Reply-To: <20230724101517.462214-1-c.heiss@proxmox.com>

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





  parent reply	other threads:[~2023-07-24 10:16 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 ` Christoph Heiss [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230724101517.462214-5-c.heiss@proxmox.com \
    --to=c.heiss@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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