[pve-devel] [PATCH pve-installer 1/6] auto: add early answer file sanity check for RAID configurations

[Christoph Heiss]

Thanks for tackling this!

On Tue Apr 22, 2025 at 6:27 PM CEST, Michael Köppl wrote:
> [..]
> +pub fn verify_disks_settings(answer: &Answer) -> Result<()> {
> +    if let DiskSelection::Selection(selection) = &answer.disks.disk_selection {
> +        let min_disks = answer.disks.fs_type.get_min_disks();
> +        if selection.len() < min_disks {
> +            bail!(
> +                "{} requires at least {} disks",
> +                answer.disks.fs_type,
> +                min_disks
> +            );
> +        }
> +    }

Perhaps another, pretty simple but useful check here would be if all
disks are unique, i.e. that there are no duplicates in the list?

> [..]
> diff --git a/proxmox-installer-common/src/options.rs b/proxmox-installer-common/src/options.rs
> index 9cc4ee0..9271b8b 100644
> --- a/proxmox-installer-common/src/options.rs
> +++ b/proxmox-installer-common/src/options.rs
> @@ -48,6 +58,19 @@ pub enum ZfsRaidLevel {
>      RaidZ3,
>  }
>
> +impl ZfsRaidLevel {
> +    pub fn get_min_disks(&self) -> usize {
> +        match self {
> +            ZfsRaidLevel::Raid0 => 1,
> +            ZfsRaidLevel::Raid1 => 2,
> +            ZfsRaidLevel::Raid10 => 4,
> +            ZfsRaidLevel::RaidZ => 3,
> +            ZfsRaidLevel::RaidZ2 => 4,
> +            ZfsRaidLevel::RaidZ3 => 5,

ZFS actually lets one create RAIDZ{1,2,3} pools with 2, 3 and 4 disks,
respectively. While maybe not really _that_ practical for real-world
usecases (starting with the overhead), do we want to still allow it?

> +        }
> +    }
> +}
> +