[pve-devel] [PATCH installer v4 2/8] move RAID setup checks to RAID level enum implementations
Michael Köppl
m.koeppl at proxmox.com
Fri Jul 11 18:27:05 CEST 2025
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. Additionally, also move the unit
tests accordingly.
Signed-off-by: Michael Köppl <m.koeppl at proxmox.com>
---
proxmox-installer-common/src/disk_checks.rs | 163 +-------------------
proxmox-installer-common/src/options.rs | 115 ++++++++++++++
proxmox-tui-installer/src/views/bootdisk.rs | 13 +-
3 files changed, 130 insertions(+), 161 deletions(-)
diff --git a/proxmox-installer-common/src/disk_checks.rs b/proxmox-installer-common/src/disk_checks.rs
index ecc43bd..62d5eab 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,108 +49,12 @@ 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 super::*;
- 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: Some(512),
- }
- }
-
fn dummy_disks(num: usize) -> Vec<Disk> {
- (0..num).map(dummy_disk).collect()
+ (0..num).map(Disk::dummy).collect()
}
#[test]
@@ -158,13 +62,13 @@ mod tests {
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),
+ Disk::dummy(0),
+ Disk::dummy(1),
+ Disk::dummy(2),
+ Disk::dummy(2),
+ Disk::dummy(3),
]),
- Err(&dummy_disk(2)),
+ Err(&Disk::dummy(2)),
);
}
@@ -189,55 +93,4 @@ mod tests {
assert!(check_disks_4kn_legacy_boot(BootType::Efi, &disks).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() {
- 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());
- }
}
diff --git a/proxmox-installer-common/src/options.rs b/proxmox-installer-common/src/options.rs
index 9271b8b..30418c2 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_raid_disks_setup(&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_raid_disks_setup(&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);
@@ -316,6 +375,19 @@ pub struct Disk {
pub block_size: Option<usize>,
}
+impl Disk {
+ #[cfg(test)]
+ pub fn dummy(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: Some(512),
+ }
+ }
+}
+
impl fmt::Display for Disk {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
// TODO: Format sizes properly with `proxmox-human-byte` once merged
@@ -542,6 +614,49 @@ mod tests {
use std::collections::BTreeMap;
use std::net::{IpAddr, Ipv4Addr};
+ fn dummy_disks(num: usize) -> Vec<Disk> {
+ (0..num).map(Disk::dummy).collect()
+ }
+
+ #[test]
+ fn btrfs_raid() {
+ let disks = dummy_disks(10);
+
+ let btrfs_raid_variants = [
+ BtrfsRaidLevel::Raid0,
+ BtrfsRaidLevel::Raid1,
+ BtrfsRaidLevel::Raid10,
+ ];
+
+ for v in btrfs_raid_variants {
+ assert!(v.check_raid_disks_setup(&[]).is_err());
+ assert!(v.check_raid_disks_setup(&disks[..v.get_min_disks() - 1]).is_err());
+ assert!(v.check_raid_disks_setup(&disks[..v.get_min_disks()]).is_ok());
+ assert!(v.check_raid_disks_setup(&disks).is_ok());
+ }
+ }
+
+ #[test]
+ fn zfs_raid() {
+ let disks = dummy_disks(10);
+
+ 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_raid_disks_setup(&[]).is_err());
+ assert!(v.check_raid_disks_setup(&disks[..v.get_min_disks() - 1]).is_err());
+ assert!(v.check_raid_disks_setup(&disks[..v.get_min_disks()]).is_ok());
+ assert!(v.check_raid_disks_setup(&disks).is_ok());
+ }
+ }
+
fn mock_setup_network() -> (SetupInfo, NetworkInfo) {
let mut interfaces = BTreeMap::new();
interfaces.insert(
diff --git a/proxmox-tui-installer/src/views/bootdisk.rs b/proxmox-tui-installer/src/views/bootdisk.rs
index 313a3c9..6f3478f 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_raid_disks_setup(&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_raid_disks_setup(&disks)
+ .map_err(|err| format!("{fstype}: {err}"))?;
}
Ok(BootdiskOptions {
--
2.47.2
More information about the pve-devel
mailing list