After testing this change and thinking about the maxroot change again, $hdsize / 4 doesn't really make sense. E.g. for an (unrealistically small, but still) disk of 8 GiB; if its unset, pve-root will be ~6.5 GiB in size, with the limit of 2 GiB, the installation fails due to ENOSPACE.
The default calculations try really hard to make installations possible even on small disks, in Proxmox/Install.pm:create_lvm_volumes() So I'm not sure if we really should restrict it that much, or rather relax it in the documentation. On Tue Apr 29, 2025 at 4:09 PM CEST, Michael Köppl wrote: > Check that the configured swapsize is not greater than hdsize / 8 as > stated in the admin guide [0]. Additionally check that maxroot is at > most hdsize / 4. Define the behavior for the auto-installer as well as > the TUI and GUI installers. > > [0] https://pve.proxmox.com/pve-docs/pve-admin-guide.html#advanced_lvm_options > > Signed-off-by: Michael Köppl <m.koe...@proxmox.com> > --- > Note regarding the change around set_hdsize in proxinstall: Before this > change, the hdsize was only set if the user manually changed it. If the > user did not change it, any checks against hdsize would check against > undefined. > > Proxmox/Install.pm | 16 ++++++ > proxinstall | 5 +- > proxmox-auto-installer/src/utils.rs | 16 ++++++ > proxmox-auto-installer/tests/parse-answer.rs | 4 +- > .../lvm_maxroot_greater_than_maximum.json | 3 ++ > .../lvm_maxroot_greater_than_maximum.toml | 16 ++++++ > .../lvm_swapsize_greater_than_hdsize.json | 3 ++ > .../lvm_swapsize_greater_than_hdsize.toml | 16 ++++++ > proxmox-installer-common/src/disk_checks.rs | 52 ++++++++++++++++++- > proxmox-tui-installer/src/views/bootdisk.rs | 6 ++- > 10 files changed, 133 insertions(+), 4 deletions(-) > create mode 100644 > proxmox-auto-installer/tests/resources/parse_answer_fail/lvm_maxroot_greater_than_maximum.json > create mode 100644 > proxmox-auto-installer/tests/resources/parse_answer_fail/lvm_maxroot_greater_than_maximum.toml > create mode 100644 > proxmox-auto-installer/tests/resources/parse_answer_fail/lvm_swapsize_greater_than_hdsize.json > create mode 100644 > proxmox-auto-installer/tests/resources/parse_answer_fail/lvm_swapsize_greater_than_hdsize.toml > > diff --git a/Proxmox/Install.pm b/Proxmox/Install.pm > index f673604..7c55d1f 100644 > --- a/Proxmox/Install.pm > +++ b/Proxmox/Install.pm > @@ -592,6 +592,22 @@ sub compute_swapsize { > return $swapsize_kb; > } > > +sub swapsize_check { > + my ($hdsize) = @_; > + my $swapsize = Proxmox::Install::Config::get_swapsize(); > + my $threshold = $hdsize / 8; > + die "swap size ${swapsize} GiB cannot be greater than ${threshold} GiB > (hard disk size / 8)\n" > + if $swapsize > $threshold; > +} > + > +sub maxroot_size_check { > + my ($hdsize) = @_; > + my $maxroot = Proxmox::Install::Config::get_maxroot(); > + my $threshold = $hdsize / 4; > + die "maximum root volume size ${maxroot} GiB cannot be greater than > ${threshold} GiB (hard disk size / 4)\n" > + if $maxroot > $threshold; > +} > + > my sub chroot_chown { > my ($root, $path, %param) = @_; > > diff --git a/proxinstall b/proxinstall > index bc9ade6..e9ff6e8 100755 > --- a/proxinstall > +++ b/proxinstall > @@ -1406,7 +1406,7 @@ sub create_hdoption_view { > > my $tmp; > > - if (($tmp = &$get_float($spinbutton_hdsize)) && ($tmp != $hdsize)) { > + if (defined($tmp = &$get_float($spinbutton_hdsize))) { > Proxmox::Install::Config::set_hdsize($tmp); > } else { > Proxmox::Install::Config::set_hdsize(undef); > @@ -1521,9 +1521,12 @@ sub create_hdsel_view { > $target_hds = [ map { $_->[1] } @$devlist ]; > } else { > my $target_hd = Proxmox::Install::Config::get_target_hd(); > + my $hdsize = Proxmox::Install::Config::get_hdsize(); > eval { > my $target_block_size = > Proxmox::Sys::Block::logical_blocksize($target_hd); > Proxmox::Install::legacy_bios_4k_check($target_block_size); > + Proxmox::Install::swapsize_check($hdsize); > + Proxmox::Install::maxroot_size_check($hdsize); > }; > if (my $err = $@) { > Proxmox::UI::message("Warning: $err\n"); > diff --git a/proxmox-auto-installer/src/utils.rs > b/proxmox-auto-installer/src/utils.rs > index d6bc6e3..75696bd 100644 > --- a/proxmox-auto-installer/src/utils.rs > +++ b/proxmox-auto-installer/src/utils.rs > @@ -13,6 +13,7 @@ use crate::{ > }; > use proxmox_installer_common::{ > ROOT_PASSWORD_MIN_LENGTH, > + disk_checks::{check_maxroot, check_swapsize}, > options::{FsType, NetworkOptions, ZfsChecksumOption, ZfsCompressOption, > email_validate}, > setup::{ > InstallBtrfsOption, InstallConfig, InstallFirstBootSetup, > InstallRootPassword, > @@ -396,6 +397,21 @@ pub fn verify_disks_settings(answer: &Answer) -> > Result<()> { > ); > } > } > + > + if let answer::FsOptions::LVM(lvm) = &answer.disks.fs_options { > + if let Some((swapsize, hdsize)) = lvm.swapsize.zip(lvm.hdsize) { > + if let Err(err) = check_swapsize(swapsize, hdsize) { > + bail!(err); > + } > + } > + > + if let Some((maxroot, hdsize)) = lvm.maxroot.zip(lvm.hdsize) { > + if let Err(err) = check_maxroot(maxroot, hdsize) { > + bail!(err); > + } > + } > + } > + > Ok(()) > } > > diff --git a/proxmox-auto-installer/tests/parse-answer.rs > b/proxmox-auto-installer/tests/parse-answer.rs > index 92dba63..e615672 100644 > --- a/proxmox-auto-installer/tests/parse-answer.rs > +++ b/proxmox-auto-installer/tests/parse-answer.rs > @@ -7,7 +7,7 @@ use proxmox_auto_installer::udevinfo::UdevInfo; > use proxmox_auto_installer::utils::parse_answer; > > use proxmox_installer_common::setup::{ > - LocaleInfo, RuntimeInfo, SetupInfo, load_installer_setup_files, > read_json, > + load_installer_setup_files, read_json, LocaleInfo, RuntimeInfo, > SetupInfo, > }; > > fn get_test_resource_path() -> Result<PathBuf, String> { > @@ -145,6 +145,8 @@ mod tests { > btrfs_raid_single_disk, > fqdn_from_dhcp_no_default_domain, > fqdn_hostname_only, > + lvm_maxroot_greater_than_maximum, > + lvm_swapsize_greater_than_hdsize, > no_fqdn_from_dhcp, > no_root_password_set, > short_password, > diff --git > a/proxmox-auto-installer/tests/resources/parse_answer_fail/lvm_maxroot_greater_than_maximum.json > > b/proxmox-auto-installer/tests/resources/parse_answer_fail/lvm_maxroot_greater_than_maximum.json > new file mode 100644 > index 0000000..bab12e6 > --- /dev/null > +++ > b/proxmox-auto-installer/tests/resources/parse_answer_fail/lvm_maxroot_greater_than_maximum.json > @@ -0,0 +1,3 @@ > +{ > + "error": "Maximum root volume size 8.01 GiB cannot be greater than 8 GiB > (hard disk size / 4)" > +} > diff --git > a/proxmox-auto-installer/tests/resources/parse_answer_fail/lvm_maxroot_greater_than_maximum.toml > > b/proxmox-auto-installer/tests/resources/parse_answer_fail/lvm_maxroot_greater_than_maximum.toml > new file mode 100644 > index 0000000..e934d29 > --- /dev/null > +++ > b/proxmox-auto-installer/tests/resources/parse_answer_fail/lvm_maxroot_greater_than_maximum.toml > @@ -0,0 +1,16 @@ > +[global] > +keyboard = "de" > +country = "at" > +fqdn = "btrfs-raid-single-disk.fail.testinstall" > +mailto = "mail@no.invalid" > +timezone = "Europe/Vienna" > +root-password = "12345678" > + > +[network] > +source = "from-dhcp" > + > +[disk-setup] > +filesystem = "ext4" > +lvm.maxroot = 8.01 > +lvm.hdsize = 32 > +disk-list = ["sda"] > diff --git > a/proxmox-auto-installer/tests/resources/parse_answer_fail/lvm_swapsize_greater_than_hdsize.json > > b/proxmox-auto-installer/tests/resources/parse_answer_fail/lvm_swapsize_greater_than_hdsize.json > new file mode 100644 > index 0000000..aa4f7fe > --- /dev/null > +++ > b/proxmox-auto-installer/tests/resources/parse_answer_fail/lvm_swapsize_greater_than_hdsize.json > @@ -0,0 +1,3 @@ > +{ > + "error": "Swap size 4.01 GiB cannot be greater than 4 GiB (hard disk size > / 8)" > +} > diff --git > a/proxmox-auto-installer/tests/resources/parse_answer_fail/lvm_swapsize_greater_than_hdsize.toml > > b/proxmox-auto-installer/tests/resources/parse_answer_fail/lvm_swapsize_greater_than_hdsize.toml > new file mode 100644 > index 0000000..ffe16dc > --- /dev/null > +++ > b/proxmox-auto-installer/tests/resources/parse_answer_fail/lvm_swapsize_greater_than_hdsize.toml > @@ -0,0 +1,16 @@ > +[global] > +keyboard = "de" > +country = "at" > +fqdn = "btrfs-raid-single-disk.fail.testinstall" > +mailto = "mail@no.invalid" > +timezone = "Europe/Vienna" > +root-password = "12345678" > + > +[network] > +source = "from-dhcp" > + > +[disk-setup] > +filesystem = "ext4" > +lvm.swapsize = 4.01 > +lvm.hdsize = 32 > +disk-list = ["sda"] > diff --git a/proxmox-installer-common/src/disk_checks.rs > b/proxmox-installer-common/src/disk_checks.rs > index d535837..77104ae 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::Disk; > +use crate::options::{Disk, LvmBootdiskOptions}; > use crate::setup::BootType; > > /// Checks a list of disks for duplicate entries, using their index as key. > @@ -49,6 +49,56 @@ pub fn check_disks_4kn_legacy_boot(boot_type: BootType, > disks: &[Disk]) -> Resul > Ok(()) > } > > +/// Checks whether the configured swap size exceeds the allowed threshold. > +/// > +/// # Arguments > +/// > +/// * `swapsize` - The size of the swap in GiB > +/// * `hdsize` - The total size of the hard disk in GiB > +pub fn check_swapsize(swapsize: f64, hdsize: f64) -> Result<(), String> { > + let threshold = hdsize / 8.0; > + if swapsize > threshold { > + return Err(format!( > + "Swap size {swapsize} GiB cannot be greater than {threshold} GiB > (hard disk size / 8)", > + )); > + } > + Ok(()) > +} > + > +/// Checks whether the configured root volume size exceeds the allowed > threshold. > +/// > +/// # Arguments > +/// > +/// * `maxroot` - The size of the root volume in GiB > +/// * `hdsize` - The total size of the hard disk in GiB > +pub fn check_maxroot(maxroot: f64, hdsize: f64) -> Result<(), String> { > + let threshold = hdsize / 4.0; > + if maxroot > threshold { > + return Err(format!( > + "Maximum root volume size {maxroot} GiB cannot be greater than > {threshold} GiB (hard disk size / 4)", > + )); > + } > + Ok(()) > +} > + > +/// Checks whether a user-supplied LVM setup is valid or not, such as the > swapsize or maxroot not > +/// exceeding certain thresholds. > +/// > +/// # Arguments > +/// > +/// * `bootdisk_opts` - The LVM options set by the user. > +pub fn check_lvm_bootdisk_opts(bootdisk_opts: &LvmBootdiskOptions) -> > Result<(), String> { > + if let Some(swap_size) = bootdisk_opts.swap_size { > + check_swapsize(swap_size, bootdisk_opts.total_size)?; > + } > + > + if let Some(max_root_size) = bootdisk_opts.max_root_size { > + check_maxroot(max_root_size, bootdisk_opts.total_size)?; > + } > + > + Ok(()) > +} > + > #[cfg(test)] > mod tests { > use crate::options::{BtrfsRaidLevel, ZfsRaidLevel}; > diff --git a/proxmox-tui-installer/src/views/bootdisk.rs > b/proxmox-tui-installer/src/views/bootdisk.rs > index e87b040..b94cf38 100644 > --- a/proxmox-tui-installer/src/views/bootdisk.rs > +++ b/proxmox-tui-installer/src/views/bootdisk.rs > @@ -17,7 +17,9 @@ use crate::InstallerState; > use crate::options::FS_TYPES; > > use proxmox_installer_common::{ > - disk_checks::{check_disks_4kn_legacy_boot, check_for_duplicate_disks}, > + disk_checks::{ > + check_disks_4kn_legacy_boot, check_for_duplicate_disks, > check_lvm_bootdisk_opts, > + }, > options::{ > AdvancedBootdiskOptions, BTRFS_COMPRESS_OPTIONS, BootdiskOptions, > BtrfsBootdiskOptions, > Disk, FsType, LvmBootdiskOptions, ZFS_CHECKSUM_OPTIONS, > ZFS_COMPRESS_OPTIONS, > @@ -261,6 +263,8 @@ impl AdvancedBootdiskOptionsView { > .get_values() > .ok_or("Failed to retrieve advanced bootdisk options")?; > > + check_lvm_bootdisk_opts(&advanced).map_err(|err| > format!("{fstype}: {err}"))?; > + > Ok(BootdiskOptions { > disks: vec![disk], > fstype, _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
