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 3FFBE1FF390 for ; Fri, 24 May 2024 12:30:26 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 0FB6F6FDC; Fri, 24 May 2024 12:30:43 +0200 (CEST) From: Christoph Heiss To: pve-devel@lists.proxmox.com Date: Fri, 24 May 2024 12:29:48 +0200 Message-ID: <20240524102949.635398-3-c.heiss@proxmox.com> X-Mailer: git-send-email 2.44.0 In-Reply-To: <20240524102949.635398-1-c.heiss@proxmox.com> References: <20240524102949.635398-1-c.heiss@proxmox.com> MIME-Version: 1.0 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.007 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 Subject: [pve-devel] [PATCH installer v4 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 v3 -> v4: * rebased on latest master, trivial conflict Changes v2 -> v3: * no changes Changes v1 -> v2: * use new placeholder functionality for non-PVE products Signed-off-by: Christoph Heiss --- proxmox-installer-common/src/options.rs | 3 +- proxmox-tui-installer/src/views/bootdisk.rs | 42 ++++++++++++--------- proxmox-tui-installer/src/views/mod.rs | 1 - 3 files changed, 26 insertions(+), 20 deletions(-) diff --git a/proxmox-installer-common/src/options.rs b/proxmox-installer-common/src/options.rs index e77914b..0534ace 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 f6fdb31..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)) @@ -596,13 +607,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) @@ -624,21 +629,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.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. + 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 5e5f4fb..eab258c 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.44.0 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel