From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 6080593E0E for ; Wed, 7 Feb 2024 15:28:30 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 4E3F412032 for ; Wed, 7 Feb 2024 15:28:30 +0100 (CET) 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) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Wed, 7 Feb 2024 15:28:29 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 2D7A944CC6 for ; Wed, 7 Feb 2024 15:28:28 +0100 (CET) From: Christoph Heiss To: pve-devel@lists.proxmox.com Date: Wed, 7 Feb 2024 15:28:13 +0100 Message-ID: <20240207142824.2613933-3-c.heiss@proxmox.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240207142824.2613933-1-c.heiss@proxmox.com> References: <20240207142824.2613933-1-c.heiss@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.047 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 T_SCC_BODY_TEXT_LINE -0.01 - URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [bootdisk.rs, mod.rs, options.rs] Subject: [pve-devel] [PATCH installer v2 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 07 Feb 2024 14:28:30 -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 --- 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 | 48 +++++++++++++-------- proxmox-tui-installer/src/views/mod.rs | 1 - 3 files changed, 32 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..b856342 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,25 @@ impl ZfsBootdiskOptionsView { fn get_values(&mut self) -> Option<(Vec, 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::(0)?; let compress = view.get_value::, _>(1)?; let checksum = view.get_value::, _>(2)?; let copies = view.get_value::(3)?; - let disk_size = view.get_value::(disk_size_index)?; - - let arc_max = if has_arc_max { - view.get_value::(4)? - .max(ZFS_ARC_MIN_SIZE_MIB) - } else { - 0 // use built-in ZFS default value - }; + let disk_size = view.get_value::(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. + // + // TODO: Return an [`Result`] from here as well and propagate the [`Result::Err`] value + // instead of defaulting to `0`. + let arc_max = view + .get_child::(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 299517e..9e27c5f 100644 --- a/proxmox-tui-installer/src/views/mod.rs +++ b/proxmox-tui-installer/src/views/mod.rs @@ -93,7 +93,6 @@ impl NumericEditView { /// /// # 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.43.0