Instead of having parts of the RAID setup checks scattered in multiple
places, move the core of the checks to implementations of the
ZfsRaidLevel and BtrfsRaidLevel enums.
Signed-off-by: Michael Köppl <m.koe...@proxmox.com>
---
proxmox-installer-common/src/disk_checks.rs | 156 ++++----------------
proxmox-installer-common/src/options.rs | 59 ++++++++
proxmox-tui-installer/src/views/bootdisk.rs | 13 +-
3 files changed, 96 insertions(+), 132 deletions(-)
diff --git a/proxmox-installer-common/src/disk_checks.rs
b/proxmox-installer-common/src/disk_checks.rs
index ecc43bd..d535837 100644
--- a/proxmox-installer-common/src/disk_checks.rs
+++ b/proxmox-installer-common/src/disk_checks.rs
@@ -1,6 +1,6 @@
use std::collections::HashSet;
-use crate::options::{BtrfsRaidLevel, Disk, ZfsRaidLevel};
+use crate::options::Disk;
use crate::setup::BootType;
/// Checks a list of disks for duplicate entries, using their index as key.
@@ -49,94 +49,10 @@ pub fn check_disks_4kn_legacy_boot(boot_type: BootType,
disks: &[Disk]) -> Resul
Ok(())
}
-/// Checks whether a user-supplied ZFS RAID setup is valid or not, such as
disk sizes andminimum
-/// number of disks.
-///
-/// # Arguments
-///
-/// * `level` - The targeted ZFS RAID level by the user.
-/// * `disks` - List of disks designated as RAID targets.
-pub fn check_zfs_raid_config(level: ZfsRaidLevel, disks: &[Disk]) ->
Result<(), String> {
- // See also Proxmox/Install.pm:get_zfs_raid_setup()
-
- 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)?;
-
- if disks.len() % 2 != 0 {
- return Err(format!(
- "Needs an even number of disks, currently selected: {}",
- disks.len(),
- ));
- }
-
- // 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.
-pub 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(())
-}
-
#[cfg(test)]
mod tests {
+ use crate::options::{BtrfsRaidLevel, ZfsRaidLevel};
+
use super::*;
fn dummy_disk(index: usize) -> Disk {
@@ -194,50 +110,38 @@ mod tests {
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());
+ let btrfs_raid_variants = [
+ BtrfsRaidLevel::Raid0,
+ BtrfsRaidLevel::Raid1,
+ BtrfsRaidLevel::Raid10,
+ ];
+
+ for v in btrfs_raid_variants {
+ assert!(v.check_disks(&[]).is_err());
+ assert!(v.check_disks(&disks[..v.get_min_disks() - 1]).is_err());
+ assert!(v.check_disks(&disks[..v.get_min_disks()]).is_ok());
+ assert!(v.check_disks(&disks).is_ok());
+ }
}
#[test]
fn zfs_raid() {
let disks = dummy_disks(10);
- assert!(check_zfs_raid_config(ZfsRaidLevel::Raid0, &[]).is_err());
- assert!(check_zfs_raid_config(ZfsRaidLevel::Raid0,
&disks[..1]).is_ok());
- assert!(check_zfs_raid_config(ZfsRaidLevel::Raid0, &disks).is_ok());
-
- assert!(check_zfs_raid_config(ZfsRaidLevel::Raid1, &[]).is_err());
- assert!(check_zfs_raid_config(ZfsRaidLevel::Raid1,
&disks[..2]).is_ok());
- assert!(check_zfs_raid_config(ZfsRaidLevel::Raid1, &disks).is_ok());
-
- assert!(check_zfs_raid_config(ZfsRaidLevel::Raid10, &[]).is_err());
- assert!(check_zfs_raid_config(ZfsRaidLevel::Raid10,
&dummy_disks(4)).is_ok());
- assert!(check_zfs_raid_config(ZfsRaidLevel::Raid10, &disks).is_ok());
-
- assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ, &[]).is_err());
- assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ,
&disks[..2]).is_err());
- assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ,
&disks[..3]).is_ok());
- assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ, &disks).is_ok());
-
- assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ2, &[]).is_err());
- assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ2,
&disks[..3]).is_err());
- assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ2,
&disks[..4]).is_ok());
- assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ2, &disks).is_ok());
-
- assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ3, &[]).is_err());
- assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ3,
&disks[..4]).is_err());
- assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ3,
&disks[..5]).is_ok());
- assert!(check_zfs_raid_config(ZfsRaidLevel::RaidZ3, &disks).is_ok());
+ let zfs_raid_variants = [
+ ZfsRaidLevel::Raid0,
+ ZfsRaidLevel::Raid1,
+ ZfsRaidLevel::Raid10,
+ ZfsRaidLevel::RaidZ,
+ ZfsRaidLevel::RaidZ2,
+ ZfsRaidLevel::RaidZ3,
+ ];
+
+ for v in zfs_raid_variants {
+ assert!(v.check_disks(&[]).is_err());
+ assert!(v.check_disks(&disks[..v.get_min_disks() - 1]).is_err());
+ assert!(v.check_disks(&disks[..v.get_min_disks()]).is_ok());
+ assert!(v.check_disks(&disks).is_ok());
+ }
}
}
diff --git a/proxmox-installer-common/src/options.rs
b/proxmox-installer-common/src/options.rs
index 9271b8b..0552954 100644
--- a/proxmox-installer-common/src/options.rs
+++ b/proxmox-installer-common/src/options.rs
@@ -6,6 +6,7 @@ use std::str::FromStr;
use std::sync::OnceLock;
use std::{cmp, fmt};
+use crate::disk_checks::check_raid_min_disks;
use crate::setup::{LocaleInfo, NetworkInfo, RuntimeInfo, SetupInfo};
use crate::utils::{CidrAddress, Fqdn};
@@ -28,6 +29,17 @@ impl BtrfsRaidLevel {
BtrfsRaidLevel::Raid10 => 4,
}
}
+
+ /// Checks whether a user-supplied Btrfs RAID setup is valid or not, such
as minimum
+ /// number of disks.
+ ///
+ /// # Arguments
+ ///
+ /// * `disks` - List of disks designated as RAID targets.
+ pub fn check_disks(&self, disks: &[Disk]) -> Result<(), String> {
+ check_raid_min_disks(disks, self.get_min_disks())?;
+ Ok(())
+ }
}
serde_plain::derive_display_from_serialize!(BtrfsRaidLevel);
@@ -69,6 +81,53 @@ impl ZfsRaidLevel {
ZfsRaidLevel::RaidZ3 => 5,
}
}
+
+ fn check_mirror_size(&self, disk1: &Disk, disk2: &Disk) -> Result<(),
String> {
+ if (disk1.size - disk2.size).abs() > disk1.size / 10. {
+ Err(format!(
+ "Mirrored disks must have same size:\n\n * {disk1}\n *
{disk2}"
+ ))
+ } else {
+ Ok(())
+ }
+ }
+
+ /// Checks whether a user-supplied ZFS RAID setup is valid or not, such as
disk sizes andminimum
+ /// number of disks.
+ ///
+ /// # Arguments
+ ///
+ /// * `disks` - List of disks designated as RAID targets.
+ pub fn check_disks(&self, disks: &[Disk]) -> Result<(), String> {
+ check_raid_min_disks(disks, self.get_min_disks())?;
+
+ match self {
+ ZfsRaidLevel::Raid0 => {}
+ ZfsRaidLevel::Raid10 => {
+ if disks.len() % 2 != 0 {
+ return Err(format!(
+ "Needs an even number of disks, currently selected:
{}",
+ disks.len(),
+ ));
+ }
+
+ // Pairs need to have the same size
+ for i in (0..disks.len()).step_by(2) {
+ self.check_mirror_size(&disks[i], &disks[i + 1])?;
+ }
+ }
+ ZfsRaidLevel::Raid1
+ | ZfsRaidLevel::RaidZ
+ | ZfsRaidLevel::RaidZ2
+ | ZfsRaidLevel::RaidZ3 => {
+ for disk in disks {
+ self.check_mirror_size(&disks[0], disk)?;
+ }
+ }
+ }
+
+ Ok(())
+ }
}
serde_plain::derive_display_from_serialize!(ZfsRaidLevel);
diff --git a/proxmox-tui-installer/src/views/bootdisk.rs
b/proxmox-tui-installer/src/views/bootdisk.rs
index 313a3c9..e87b040 100644
--- a/proxmox-tui-installer/src/views/bootdisk.rs
+++ b/proxmox-tui-installer/src/views/bootdisk.rs
@@ -17,10 +17,7 @@ use crate::InstallerState;
use crate::options::FS_TYPES;
use proxmox_installer_common::{
- disk_checks::{
- check_btrfs_raid_config, check_disks_4kn_legacy_boot,
check_for_duplicate_disks,
- check_zfs_raid_config,
- },
+ disk_checks::{check_disks_4kn_legacy_boot, check_for_duplicate_disks},
options::{
AdvancedBootdiskOptions, BTRFS_COMPRESS_OPTIONS, BootdiskOptions,
BtrfsBootdiskOptions,
Disk, FsType, LvmBootdiskOptions, ZFS_CHECKSUM_OPTIONS,
ZFS_COMPRESS_OPTIONS,
@@ -275,7 +272,9 @@ impl AdvancedBootdiskOptionsView {
.ok_or("Failed to retrieve advanced bootdisk options")?;
if let FsType::Zfs(level) = fstype {
- check_zfs_raid_config(level, &disks).map_err(|err|
format!("{fstype}: {err}"))?;
+ level
+ .check_disks(&disks)
+ .map_err(|err| format!("{fstype}: {err}"))?;
}
Ok(BootdiskOptions {
@@ -289,7 +288,9 @@ impl AdvancedBootdiskOptionsView {
.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}"))?;
+ level
+ .check_disks(&disks)
+ .map_err(|err| format!("{fstype}: {err}"))?;
}
Ok(BootdiskOptions {
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
