Re: [pve-devel] [PATCH pve-installer 2/6] common: use get_min_disks as single source of truth for RAID config checks

["Christoph Heiss" <c.heiss@proxmox.com>]

On Tue Apr 22, 2025 at 6:27 PM CEST, Michael Köppl wrote:
> To avoid having multiple places where the minimum number of disks
> required for a certain RAID setup is defined, use get_min_disks in the
> common installer RAID config checks as well, keeping the behavior the
> same as for the answer file sanity checks.
>
> Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
> ---
> An argument could be made here that there should be a declarative single
> source of truth in the form of a JSON or similar that both Perl and
> Rust-based installers can use. I would love some input on this as I
> think it could make sense, but would require more extensive changes.
> We already use JSON files as sources for selectable values, etc. in
> other parts of the installer, so we would not be doing anything new
> here.

Thinking about it again (after our short talk off-list), I think putting
it into a separate (JSON) file might be to much work than it's really
worth. After all, there are only two places, these values won't change
and are not all that magic. So keeping separate definitions between the
Perl and Rust code is fine IMHO.

>
>  proxmox-installer-common/src/disk_checks.rs | 18 ++++--------------
>  1 file changed, 4 insertions(+), 14 deletions(-)
>
> diff --git a/proxmox-installer-common/src/disk_checks.rs b/proxmox-installer-common/src/disk_checks.rs
> index ecc43bd..1d17e2c 100644
> --- a/proxmox-installer-common/src/disk_checks.rs
> +++ b/proxmox-installer-common/src/disk_checks.rs
> @@ -69,17 +69,16 @@ pub fn check_zfs_raid_config(level: ZfsRaidLevel, disks: &[Disk]) -> Result<(),

While looking at all the disks checks, most of them could be made
methods of the respective enum, e.g. so one could do something like
`ZfsRaidLevel::Raid1.check_disks(disks)`.

What do you think? Would be a nice cleanup (via a separate series would
also be okay, of course).


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