From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <c.heiss@proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
key-exchange X25519 server-signature RSA-PSS (2048 bits))
(No client certificate requested)
by lists.proxmox.com (Postfix) with ESMTPS id B6070963B5
for <pve-devel@lists.proxmox.com>; Mon, 15 Apr 2024 15:20:45 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 9E77DA932
for <pve-devel@lists.proxmox.com>; Mon, 15 Apr 2024 15:20:45 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com
[94.136.29.106])
(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
key-exchange X25519 server-signature RSA-PSS (2048 bits))
(No client certificate requested)
by firstgate.proxmox.com (Proxmox) with ESMTPS
for <pve-devel@lists.proxmox.com>; Mon, 15 Apr 2024 15:20:44 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 5F1F744A35
for <pve-devel@lists.proxmox.com>; Mon, 15 Apr 2024 15:20:44 +0200 (CEST)
From: Christoph Heiss <c.heiss@proxmox.com>
To: pve-devel@lists.proxmox.com
Date: Mon, 15 Apr 2024 15:20:26 +0200
Message-ID: <20240415132039.1354666-3-c.heiss@proxmox.com>
X-Mailer: git-send-email 2.44.0
In-Reply-To: <20240415132039.1354666-1-c.heiss@proxmox.com>
References: <20240415132039.1354666-1-c.heiss@proxmox.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
X-SPAM-LEVEL: Spam detection results: 0
AWL -0.050 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
PROLO_LEO1 0.1 Meta Catches all Leo drug variations so far
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
Subject: [pve-devel] [PATCH installer v3 2/3] tui: expose arc size setting
for zfs bootdisks for all products
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>
X-List-Received-Date: Mon, 15 Apr 2024 13:20:45 -0000
For non-PVE products, simply use the ZFS defaults (aka. 50%) and leave
unset, if the user never touches that setting.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v2 -> v3:
* no changes
Changes v1 -> v2:
* use new placeholder functionality for non-PVE products
proxmox-installer-common/src/options.rs | 3 +-
proxmox-tui-installer/src/views/bootdisk.rs | 45 ++++++++++++---------
proxmox-tui-installer/src/views/mod.rs | 1 -
3 files changed, 29 insertions(+), 20 deletions(-)
diff --git a/proxmox-installer-common/src/options.rs b/proxmox-installer-common/src/options.rs
index 1aa8f65..a210142 100644
--- a/proxmox-installer-common/src/options.rs
+++ b/proxmox-installer-common/src/options.rs
@@ -205,7 +205,8 @@ impl ZfsBootdiskOptions {
/// 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 {
- // Use ZFS default for non-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.)
diff --git a/proxmox-tui-installer/src/views/bootdisk.rs b/proxmox-tui-installer/src/views/bootdisk.rs
index 7e13e91..3d9be50 100644
--- a/proxmox-tui-installer/src/views/bootdisk.rs
+++ b/proxmox-tui-installer/src/views/bootdisk.rs
@@ -564,7 +564,18 @@ impl ZfsBootdiskOptionsView {
options: &ZfsBootdiskOptions,
product_conf: &ProductConfig,
) -> Self {
- let is_pve = product_conf.product == ProxmoxProduct::PVE;
+ 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 {
+ view.content(options.arc_max)
+ } else {
+ view.placeholder(runinfo.total_memory / 2)
+ }
+ };
let inner = FormView::new()
.child("ashift", IntegerEditView::new().content(options.ashift))
@@ -592,14 +603,11 @@ impl ZfsBootdiskOptionsView {
.unwrap_or_default(),
),
)
- .child("copies", IntegerEditView::new().content(options.copies).max_value(3))
- .child_conditional(
- is_pve,
- "ARC max size",
- IntegerEditView::new_with_suffix("MiB")
- .max_value(runinfo.total_memory)
- .content(options.arc_max),
+ .child(
+ "copies",
+ IntegerEditView::new().content(options.copies).max_value(3),
)
+ .child("ARC max size", arc_max_view)
.child("hdsize", DiskSizeEditView::new().content(options.disk_size));
let view = MultiDiskOptionsView::new(&runinfo.disks, &options.selected_disks, inner)
@@ -621,21 +629,22 @@ impl ZfsBootdiskOptionsView {
fn get_values(&mut self) -> Option<(Vec<Disk>, ZfsBootdiskOptions)> {
let (disks, selected_disks) = self.view.get_disks_and_selection()?;
let view = self.view.inner_mut()?;
- let has_arc_max = view.len() >= 6;
- let disk_size_index = if has_arc_max { 5 } else { 4 };
let ashift = view.get_value::<IntegerEditView, _>(0)?;
let compress = view.get_value::<SelectView<_>, _>(1)?;
let checksum = view.get_value::<SelectView<_>, _>(2)?;
let copies = view.get_value::<IntegerEditView, _>(3)?;
- let disk_size = view.get_value::<DiskSizeEditView, _>(disk_size_index)?;
-
- let arc_max = if has_arc_max {
- view.get_value::<IntegerEditView, _>(4)?
- .max(ZFS_ARC_MIN_SIZE_MIB)
- } else {
- 0 // use built-in ZFS default value
- };
+ let disk_size = view.get_value::<DiskSizeEditView, _>(5)?;
+
+ // If a value is set, return that and clamp it to at least [`ZFS_ARC_MIN_SIZE_MIB`].
+ //
+ // Otherwise, if no value was set or an error occured return `0`. The former simply means
+ // that the placeholder value is still there.
+ let arc_max = view
+ .get_child::<IntegerEditView>(4)?
+ .get_content_maybe()
+ .map_or(Ok(0), |v| v.map(|v| v.max(ZFS_ARC_MIN_SIZE_MIB)))
+ .unwrap_or(0);
Some((
disks,
diff --git a/proxmox-tui-installer/src/views/mod.rs b/proxmox-tui-installer/src/views/mod.rs
index 5e5f4fb..eab258c 100644
--- a/proxmox-tui-installer/src/views/mod.rs
+++ b/proxmox-tui-installer/src/views/mod.rs
@@ -93,7 +93,6 @@ impl<T: Copy + ToString + FromStr + PartialOrd> NumericEditView<T> {
///
/// # Arguments
/// `placeholder` - The placeholder value to set for this view.
- #[allow(unused)]
pub fn placeholder(mut self, placeholder: T) -> Self {
self.placeholder = Some(placeholder);
self.allow_empty(true)
--
2.44.0