From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
by lore.proxmox.com (Postfix) with ESMTPS id 9DA311FF164
for <inbox@lore.proxmox.com>; Fri, 28 Feb 2025 10:43:59 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id DEF0E9982;
Fri, 28 Feb 2025 10:43:53 +0100 (CET)
From: Christoph Heiss <c.heiss@proxmox.com>
To: pve-devel@lists.proxmox.com
Date: Fri, 28 Feb 2025 10:43:37 +0100
Message-ID: <20250228094341.147783-3-c.heiss@proxmox.com>
X-Mailer: git-send-email 2.47.1
In-Reply-To: <20250228094341.147783-1-c.heiss@proxmox.com>
References: <20250228094341.147783-1-c.heiss@proxmox.com>
MIME-Version: 1.0
X-SPAM-LEVEL: Spam detection results: 0
AWL -0.022 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
DMARC_MISSING 0.1 Missing DMARC policy
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
URIBL_SBL_A 0.1 Contains URL's A record listed in the Spamhaus SBL blocklist
[185.199.110.153, 185.199.111.153]
Subject: [pve-devel] [PATCH installer 2/4] tui: use default ZFS ARC maximum
size from runtime enviroment
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.proxmox.com>
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.heiss@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