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

["Michael Köppl" <m.koeppl@proxmox.com>]

On 4/28/25 13:25, Christoph Heiss wrote:
> 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?

Good idea, I'll add that for the v2. Thanks!

> 
>> [..]
>> 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?

I personally don't like putting too many constraints on what users can 
do. Even if not every setting is practical, I think the installer should 
allow them as long as they don't mean that the whole installation is 
going to crash halfway through, especially if manually creating pools 
like that would work. Maybe someone else has an opinion on this and can 
weigh in, though. In any case, thanks for the suggestion!


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel