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
