Now that the value is pre-calculated in the low-level installer and
written to `run-env.json`, use it from there instead of calculating it
separately - thus having a single source of truth.
Signed-off-by: Christoph Heiss <c.he...@proxmox.com>
---
proxmox-installer-common/src/options.rs | 58 ++-------------------
proxmox-installer-common/src/setup.rs | 3 ++
proxmox-tui-installer/src/views/bootdisk.rs | 42 ++++++---------
3 files changed, 21 insertions(+), 82 deletions(-)
diff --git a/proxmox-installer-common/src/options.rs
b/proxmox-installer-common/src/options.rs
index 0fd3e43..40100d8 100644
--- a/proxmox-installer-common/src/options.rs
+++ b/proxmox-installer-common/src/options.rs
@@ -6,9 +6,7 @@ use std::str::FromStr;
use std::sync::OnceLock;
use std::{cmp, fmt};
-use crate::setup::{
- LocaleInfo, NetworkInfo, ProductConfig, ProxmoxProduct, RuntimeInfo,
SetupInfo,
-};
+use crate::setup::{LocaleInfo, NetworkInfo, RuntimeInfo, SetupInfo};
use crate::utils::{CidrAddress, Fqdn};
#[derive(Copy, Clone, Debug, Deserialize, Serialize, Eq, PartialEq)]
@@ -256,42 +254,20 @@ pub struct ZfsBootdiskOptions {
impl ZfsBootdiskOptions {
/// Panics if the disk list is empty.
- pub fn defaults_from(runinfo: &RuntimeInfo, product_conf: &ProductConfig)
-> Self {
+ pub fn defaults_from(runinfo: &RuntimeInfo) -> Self {
let disk = &runinfo.disks[0];
Self {
ashift: 12,
compress: ZfsCompressOption::default(),
checksum: ZfsChecksumOption::default(),
copies: 1,
- arc_max: default_zfs_arc_max(product_conf.product,
runinfo.total_memory),
+ arc_max: runinfo.default_zfs_arc_max,
disk_size: disk.size,
selected_disks: (0..runinfo.disks.len()).collect(),
}
}
}
-/// Calculates the default upper limit for the ZFS ARC size.
-/// See also <https://bugzilla.proxmox.com/show_bug.cgi?id=4829> and
-///
https://openzfs.github.io/openzfs-docs/Performance%20and%20Tuning/Module%20Parameters.html#zfs-arc-max
-///
-/// # Arguments
-/// * `product` - The product to be installed
-/// * `total_memory` - Total memory installed in the system, in MiB
-///
-/// # Returns
-/// The default ZFS maximum ARC size in MiB for this system.
-fn default_zfs_arc_max(product: ProxmoxProduct, total_memory: usize) -> usize {
- if product != ProxmoxProduct::PVE {
- // For products other the PVE, just let ZFS decide on its own. Setting
`0`
- // causes the installer to skip writing the `zfs_arc_max` module
parameter.
- 0
- } else {
- ((total_memory as f64) / 10.)
- .round()
- .clamp(64., 16. * 1024.) as usize
- }
-}
-
#[derive(Clone, Debug)]
pub enum AdvancedBootdiskOptions {
Lvm(LvmBootdiskOptions),
@@ -493,31 +469,3 @@ pub fn email_validate(email: &str) -> Result<()> {
Ok(())
}
-
-#[cfg(test)]
-mod tests {
- use super::*;
-
- #[test]
- fn zfs_arc_limit() {
- const TESTS: &[(usize, usize)] = &[
- (16, 64), // at least 64 MiB
- (1024, 102),
- (4 * 1024, 410),
- (8 * 1024, 819),
- (150 * 1024, 15360),
- (160 * 1024, 16384),
- (1024 * 1024, 16384), // maximum of 16 GiB
- ];
-
- for (total_memory, expected) in TESTS {
- assert_eq!(
- default_zfs_arc_max(ProxmoxProduct::PVE, *total_memory),
- *expected
- );
- assert_eq!(default_zfs_arc_max(ProxmoxProduct::PBS,
*total_memory), 0);
- assert_eq!(default_zfs_arc_max(ProxmoxProduct::PMG,
*total_memory), 0);
- assert_eq!(default_zfs_arc_max(ProxmoxProduct::PDM,
*total_memory), 0);
- }
- }
-}
diff --git a/proxmox-installer-common/src/setup.rs
b/proxmox-installer-common/src/setup.rs
index 93818f3..6b033e1 100644
--- a/proxmox-installer-common/src/setup.rs
+++ b/proxmox-installer-common/src/setup.rs
@@ -390,6 +390,9 @@ pub struct RuntimeInfo {
/// Whether the system was booted with SecureBoot enabled
#[serde(default, deserialize_with = "deserialize_bool_from_int_maybe")]
pub secure_boot: Option<bool>,
+
+ /// Default upper limit for the ZFS ARC size, in MiB.
+ pub default_zfs_arc_max: usize,
}
#[derive(Copy, Clone, Eq, Deserialize, PartialEq, Serialize)]
diff --git a/proxmox-tui-installer/src/views/bootdisk.rs
b/proxmox-tui-installer/src/views/bootdisk.rs
index fffb05e..2e2019d 100644
--- a/proxmox-tui-installer/src/views/bootdisk.rs
+++ b/proxmox-tui-installer/src/views/bootdisk.rs
@@ -162,7 +162,7 @@ impl AdvancedBootdiskOptionsView {
&product_conf,
)),
AdvancedBootdiskOptions::Zfs(zfs) => {
- view.add_child(ZfsBootdiskOptionsView::new(runinfo, zfs,
&product_conf))
+ view.add_child(ZfsBootdiskOptionsView::new(runinfo, zfs))
}
AdvancedBootdiskOptions::Btrfs(btrfs) => {
view.add_child(BtrfsBootdiskOptionsView::new(runinfo, btrfs))
@@ -213,10 +213,9 @@ impl AdvancedBootdiskOptionsView {
&product_conf,
))
}
- FsType::Zfs(_) =>
view.add_child(ZfsBootdiskOptionsView::new_with_defaults(
- &runinfo,
- &product_conf,
- )),
+ FsType::Zfs(_) => {
+
view.add_child(ZfsBootdiskOptionsView::new_with_defaults(&runinfo))
+ }
FsType::Btrfs(_) => {
view.add_child(BtrfsBootdiskOptionsView::new_with_defaults(&runinfo))
}
@@ -631,27 +630,20 @@ struct ZfsBootdiskOptionsView {
impl ZfsBootdiskOptionsView {
// TODO: Re-apply previous disk selection from `options` correctly
- fn new(
- runinfo: &RuntimeInfo,
- options: &ZfsBootdiskOptions,
- product_conf: &ProductConfig,
- ) -> Self {
+ fn new(runinfo: &RuntimeInfo, options: &ZfsBootdiskOptions) -> Self {
let arc_max_view = {
let view =
IntegerEditView::new_with_suffix("MiB").max_value(runinfo.total_memory);
- // For PVE "force" the default value, for other products place the
recommended value
- // only in the placeholder. This causes for the latter to not
write the module option
- // if the value is never modified by the user.
- if product_conf.product == ProxmoxProduct::PVE {
+ // If the runtime environment provides a non-zero value, that is
+ // also not the built-in ZFS default of half the system memory, use
+ // that as default.
+ // Otherwise, just place the ZFS default into the placeholder.
+ if runinfo.default_zfs_arc_max > 0
+ && runinfo.default_zfs_arc_max != runinfo.total_memory / 2
+ {
view.content(options.arc_max)
} else {
- let view = view.placeholder(runinfo.total_memory / 2);
-
- if options.arc_max != 0 {
- view.content(options.arc_max)
- } else {
- view
- }
+ view.placeholder(runinfo.total_memory / 2)
}
};
@@ -696,12 +688,8 @@ impl ZfsBootdiskOptionsView {
Self { view }
}
- fn new_with_defaults(runinfo: &RuntimeInfo, product_conf: &ProductConfig)
-> Self {
- Self::new(
- runinfo,
- &ZfsBootdiskOptions::defaults_from(runinfo, product_conf),
- product_conf,
- )
+ fn new_with_defaults(runinfo: &RuntimeInfo) -> Self {
+ Self::new(runinfo, &ZfsBootdiskOptions::defaults_from(runinfo))
}
fn get_values(&mut self) -> Option<(Vec<Disk>, ZfsBootdiskOptions)> {
--
2.47.1
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
