From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id D944A1FF16A for ; Wed, 14 Aug 2024 15:25:42 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 7A2231E48B; Wed, 14 Aug 2024 15:25:56 +0200 (CEST) From: Christoph Heiss To: pve-devel@lists.proxmox.com Date: Wed, 14 Aug 2024 15:25:40 +0200 Message-ID: <20240814132542.1315840-3-c.heiss@proxmox.com> X-Mailer: git-send-email 2.45.2 In-Reply-To: <20240814132542.1315840-1-c.heiss@proxmox.com> References: <20240814132542.1315840-1-c.heiss@proxmox.com> MIME-Version: 1.0 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.028 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 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. 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. [options.rs, bootdisk.rs, mod.rs] Subject: [pve-devel] [PATCH installer v5 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: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" 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 v4 -> v5: * rebased on latest master * fixed value not persisting across dialog open/close for non-PVE Changes v3 -> v4: * rebased on latest master 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 | 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 9375ded..34dfadb 100644 --- a/proxmox-installer-common/src/options.rs +++ b/proxmox-installer-common/src/options.rs @@ -214,7 +214,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 1e22105..dfb1608 100644 --- a/proxmox-tui-installer/src/views/bootdisk.rs +++ b/proxmox-tui-installer/src/views/bootdisk.rs @@ -622,7 +622,24 @@ 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 { + let view = view.placeholder(runinfo.total_memory / 2); + + if options.arc_max != 0 { + view.content(options.arc_max) + } else { + view + } + } + }; let inner = FormView::new() .child("ashift", IntegerEditView::new().content(options.ashift)) @@ -654,13 +671,7 @@ impl ZfsBootdiskOptionsView { "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("ARC max size", arc_max_view) .child("hdsize", DiskSizeEditView::new().content(options.disk_size)); let view = MultiDiskOptionsView::new(&runinfo.disks, &options.selected_disks, inner) @@ -682,21 +693,22 @@ impl ZfsBootdiskOptionsView { fn get_values(&mut self) -> Option<(Vec, ZfsBootdiskOptions)> { let (disks, selected_disks) = self.view.get_disks_and_selection()?; let view = self.view.get_options_view()?; - 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 occurred return `0`. The former simply means + // that the placeholder value is still there. + 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 c7491fc..a41dac3 100644 --- a/proxmox-tui-installer/src/views/mod.rs +++ b/proxmox-tui-installer/src/views/mod.rs @@ -96,7 +96,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.45.2 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel