[pve-devel] [PATCH installer v4 3/8] close #5887: add sanity check for LVM swapsize

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

Check that the configured swapsize is not greater than hdsize / 8 as
stated in the admin guide [0]. 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.koeppl@proxmox.com>
---
 Proxmox/Install.pm                            |  8 +++++
 proxinstall                                   |  4 ++-
 proxmox-auto-installer/src/utils.rs           |  8 +++++
 proxmox-auto-installer/tests/parse-answer.rs  |  1 +
 .../lvm_swapsize_greater_than_hdsize.json     |  3 ++
 .../lvm_swapsize_greater_than_hdsize.toml     | 16 +++++++++
 proxmox-installer-common/src/disk_checks.rs   | 33 ++++++++++++++++++-
 proxmox-tui-installer/Cargo.toml              |  1 +
 proxmox-tui-installer/src/views/bootdisk.rs   | 10 +++++-
 9 files changed, 81 insertions(+), 3 deletions(-)
 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 95b7b30..f852147 100644
--- a/Proxmox/Install.pm
+++ b/Proxmox/Install.pm
@@ -599,6 +599,14 @@ 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;
+}
+
 my sub chroot_chown {
     my ($root, $path, %param) = @_;
 
diff --git a/proxinstall b/proxinstall
index 904668e..84f1a91 100755
--- a/proxinstall
+++ b/proxinstall
@@ -1488,7 +1488,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);
@@ -1607,9 +1607,11 @@ 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);
                 };
                 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 ad2db84..24ff4ea 100644
--- a/proxmox-auto-installer/src/utils.rs
+++ b/proxmox-auto-installer/src/utils.rs
@@ -12,6 +12,7 @@ use crate::{
 };
 use proxmox_installer_common::{
     ROOT_PASSWORD_MIN_LENGTH,
+    disk_checks::check_swapsize,
     options::{FsType, NetworkOptions, ZfsChecksumOption, ZfsCompressOption, email_validate},
     setup::{
         InstallBtrfsOption, InstallConfig, InstallFirstBootSetup, InstallRootPassword,
@@ -397,6 +398,13 @@ 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) {
+            check_swapsize(swapsize, hdsize)?;
+        }
+    }
+
     Ok(())
 }
 
diff --git a/proxmox-auto-installer/tests/parse-answer.rs b/proxmox-auto-installer/tests/parse-answer.rs
index 92dba63..ca8c09f 100644
--- a/proxmox-auto-installer/tests/parse-answer.rs
+++ b/proxmox-auto-installer/tests/parse-answer.rs
@@ -145,6 +145,7 @@ mod tests {
             btrfs_raid_single_disk,
             fqdn_from_dhcp_no_default_domain,
             fqdn_hostname_only,
+            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_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 16b488e..b8b9c46 100644
--- a/proxmox-installer-common/src/disk_checks.rs
+++ b/proxmox-installer-common/src/disk_checks.rs
@@ -1,6 +1,8 @@
 use std::collections::HashSet;
 
-use crate::options::Disk;
+use anyhow::ensure;
+
+use crate::options::{Disk, LvmBootdiskOptions};
 use crate::setup::BootType;
 
 /// Checks a list of disks for duplicate entries, using their index as key.
@@ -49,6 +51,35 @@ 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) -> anyhow::Result<()> {
+    let threshold = hdsize / 8.0;
+    ensure!(
+        swapsize <= threshold,
+        "Swap size {swapsize} GiB cannot be greater than {threshold} GiB (hard disk size / 8)"
+    );
+    Ok(())
+}
+
+/// Checks whether a user-supplied LVM setup is valid or not, such as the swapsize not
+/// exceeding a certain threshold.
+///
+/// # Arguments
+///
+/// * `bootdisk_opts` - The LVM options set by the user.
+pub fn check_lvm_bootdisk_opts(bootdisk_opts: &LvmBootdiskOptions) -> anyhow::Result<()> {
+    if let Some(swap_size) = bootdisk_opts.swap_size {
+        check_swapsize(swap_size, bootdisk_opts.total_size)?;
+    }
+
+    Ok(())
+}
+
 #[cfg(test)]
 mod tests {
     use crate::options::{BtrfsRaidLevel, ZfsRaidLevel};
diff --git a/proxmox-tui-installer/Cargo.toml b/proxmox-tui-installer/Cargo.toml
index 139c85c..cc2baeb 100644
--- a/proxmox-tui-installer/Cargo.toml
+++ b/proxmox-tui-installer/Cargo.toml
@@ -9,6 +9,7 @@ homepage = "https://www.proxmox.com"
 
 [dependencies]
 proxmox-installer-common.workspace = true
+anyhow.workspace = true
 serde_json.workspace = true
 
 cursive = { version = "0.21", default-features = false, features = ["crossterm-backend"] }
diff --git a/proxmox-tui-installer/src/views/bootdisk.rs b/proxmox-tui-installer/src/views/bootdisk.rs
index 6f3478f..9f6d235 100644
--- a/proxmox-tui-installer/src/views/bootdisk.rs
+++ b/proxmox-tui-installer/src/views/bootdisk.rs
@@ -3,6 +3,8 @@ use std::{
     sync::{Arc, Mutex},
 };
 
+use anyhow::Context;
+
 use cursive::{
     Cursive, Vec2, View,
     view::{Nameable, Resizable, ViewWrapper},
@@ -17,7 +19,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 +265,10 @@ impl AdvancedBootdiskOptionsView {
                 .get_values()
                 .ok_or("Failed to retrieve advanced bootdisk options")?;
 
+            check_lvm_bootdisk_opts(&advanced)
+                .context(fstype.to_string())
+                .map_err(|err| format!("{:#}", err))?;
+
             Ok(BootdiskOptions {
                 disks: vec![disk],
                 fstype,
-- 
2.47.2



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