[pve-devel] [RFC PATCH pve-installer 3/6] close #5887: add sanity check for LVM swapsize and maxroot
Michael Köppl
m.koeppl at proxmox.com
Tue Apr 22 18:27:36 CEST 2025
Check that the configured swapsize is not greater than the total size
of the disk and that maxroot is at most hdsize / 4. Define the
behavior for the auto-installer as well as the TUI and GUI installers.
Signed-off-by: Michael Köppl <m.koeppl at proxmox.com>
---
The changes implemented in this patch regarding swap size only avoid
rather obvious misconfigurations. However, users could still configure a
32GB swap volume on a 32GB hard disk and the installer would fail later
on. Users could benefit another check that ensures the swap volume size
does not go beyond a certain threshold. One option could be to set the
limit similar to the 8GB maximum when the swap volume size is calculated
from the size of the memory. OTOH, this might also be
considered too restrictive. Would love some input on this.
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 | 14 +++++++++++++
proxinstall | 5 ++++-
proxmox-auto-installer/src/utils.rs | 13 ++++++++++++
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 | 20 ++++++++++++++++++-
proxmox-tui-installer/src/views/bootdisk.rs | 4 +++-
10 files changed, 94 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..d93a54b 100644
--- a/Proxmox/Install.pm
+++ b/Proxmox/Install.pm
@@ -592,6 +592,20 @@ sub compute_swapsize {
return $swapsize_kb;
}
+sub swapsize_check {
+ my ($hdsize) = @_;
+ my $swapsize = Proxmox::Install::Config::get_swapsize();
+ die "swapsize cannot be greater than hdsize\n"
+ if $swapsize > $hdsize;
+}
+
+sub maxroot_size_check {
+ my ($hdsize) = @_;
+ my $maxroot = Proxmox::Install::Config::get_maxroot();
+ die "maxroot cannot be greater than hdsize / 4\n"
+ if $maxroot > ($hdsize/4);
+}
+
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..85a1f52 100644
--- a/proxmox-auto-installer/src/utils.rs
+++ b/proxmox-auto-installer/src/utils.rs
@@ -396,6 +396,19 @@ pub fn verify_disks_settings(answer: &Answer) -> Result<()> {
);
}
}
+
+ if let answer::FsOptions::LVM(lvm) = &answer.disks.fs_options {
+ if lvm.swapsize > lvm.hdsize {
+ bail!("LVM swapsize cannot be greater than hdsize");
+ }
+
+ if let Some((maxroot, hdsize)) = lvm.maxroot.zip(lvm.hdsize) {
+ if maxroot > hdsize / 4.0 {
+ bail!("LVM maxroot cannot be greater than hdsize / 4");
+ }
+ }
+ }
+
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..f05d6ee
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/lvm_maxroot_greater_than_maximum.json
@@ -0,0 +1,3 @@
+{
+ "error": "LVM maxroot cannot be greater than hdsize / 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 at 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..562d834
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/lvm_swapsize_greater_than_hdsize.json
@@ -0,0 +1,3 @@
+{
+ "error": "LVM swapsize cannot be greater than hdsize"
+}
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..a29c36a
--- /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 at no.invalid"
+timezone = "Europe/Vienna"
+root-password = "12345678"
+
+[network]
+source = "from-dhcp"
+
+[disk-setup]
+filesystem = "ext4"
+lvm.swapsize = 33
+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 1d17e2c..bb33baf 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::{BtrfsRaidLevel, Disk, ZfsRaidLevel};
+use crate::options::{BtrfsRaidLevel, Disk, LvmBootdiskOptions, ZfsRaidLevel};
use crate::setup::BootType;
/// Checks a list of disks for duplicate entries, using their index as key.
@@ -49,6 +49,24 @@ pub fn check_disks_4kn_legacy_boot(boot_type: BootType, disks: &[Disk]) -> Resul
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<(), &str> {
+ if bootdisk_opts.swap_size > Some(bootdisk_opts.total_size) {
+ return Err("Swap size cannot be greater than total size");
+ }
+
+ if bootdisk_opts.max_root_size > Some(bootdisk_opts.total_size / 4.0) {
+ return Err("Max root size cannot be greater than (total size / 4)");
+ }
+
+ Ok(())
+}
+
/// Checks whether a user-supplied ZFS RAID setup is valid or not, such as disk sizes andminimum
/// number of disks.
///
diff --git a/proxmox-tui-installer/src/views/bootdisk.rs b/proxmox-tui-installer/src/views/bootdisk.rs
index 313a3c9..60d5316 100644
--- a/proxmox-tui-installer/src/views/bootdisk.rs
+++ b/proxmox-tui-installer/src/views/bootdisk.rs
@@ -19,7 +19,7 @@ use crate::options::FS_TYPES;
use proxmox_installer_common::{
disk_checks::{
check_btrfs_raid_config, check_disks_4kn_legacy_boot, check_for_duplicate_disks,
- check_zfs_raid_config,
+ check_lvm_bootdisk_opts, check_zfs_raid_config,
},
options::{
AdvancedBootdiskOptions, BTRFS_COMPRESS_OPTIONS, BootdiskOptions, BtrfsBootdiskOptions,
@@ -264,6 +264,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,
--
2.39.5
More information about the pve-devel
mailing list