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)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id BAB659FF7D for ; Tue, 7 Nov 2023 13:21:11 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id A335E325D8 for ; Tue, 7 Nov 2023 13:21:10 +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)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Tue, 7 Nov 2023 13:21:08 +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 ED1C1469D5 for ; Tue, 7 Nov 2023 13:21:07 +0100 (CET) From: Christoph Heiss To: pve-devel@lists.proxmox.com Date: Tue, 7 Nov 2023 13:20:54 +0100 Message-ID: <20231107122102.732841-7-c.heiss@proxmox.com> X-Mailer: git-send-email 2.42.0 In-Reply-To: <20231107122102.732841-1-c.heiss@proxmox.com> References: <20231107122102.732841-1-c.heiss@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.014 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 T_SCC_BODY_TEXT_LINE -0.01 - Subject: [pve-devel] [PATCH installer v4 6/6] tui: bootdisk: expose `arc_max` ZFS option for PVE installations 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: Tue, 07 Nov 2023 12:21:11 -0000 To set the maximum value for arc_max accordingly, simply pass down `RuntimeInfo` directly instead of the disks array to the views. Signed-off-by: Christoph Heiss --- proxmox-tui-installer/src/main.rs | 2 +- proxmox-tui-installer/src/views/bootdisk.rs | 68 ++++++++++++++------- proxmox-tui-installer/src/views/mod.rs | 1 - 3 files changed, 47 insertions(+), 24 deletions(-) diff --git a/proxmox-tui-installer/src/main.rs b/proxmox-tui-installer/src/main.rs index 2e5a194..e561d57 100644 --- a/proxmox-tui-installer/src/main.rs +++ b/proxmox-tui-installer/src/main.rs @@ -378,7 +378,7 @@ fn bootdisk_dialog(siv: &mut Cursive) -> InstallerView { InstallerView::new( &state, - BootdiskOptionsView::new(siv, &state.runtime_info.disks, &state.options.bootdisk) + BootdiskOptionsView::new(siv, &state.runtime_info, &state.options.bootdisk) .with_name("bootdisk-options"), Box::new(|siv| { let options = siv.call_on_name("bootdisk-options", BootdiskOptionsView::get_values); diff --git a/proxmox-tui-installer/src/views/bootdisk.rs b/proxmox-tui-installer/src/views/bootdisk.rs index 3b2242a..5272258 100644 --- a/proxmox-tui-installer/src/views/bootdisk.rs +++ b/proxmox-tui-installer/src/views/bootdisk.rs @@ -25,6 +25,10 @@ use proxmox_installer_common::{ setup::{BootType, ProductConfig, ProxmoxProduct, RuntimeInfo}, }; +/// OpenZFS specifies 64 MiB as the absolute minimum: +/// https://openzfs.github.io/openzfs-docs/Performance%20and%20Tuning/Module%20Parameters.html#zfs-arc-max +const ZFS_ARC_MIN_SIZE_MIB: usize = 64; // MiB + pub struct BootdiskOptionsView { view: LinearLayout, advanced_options: Rc>, @@ -32,13 +36,13 @@ pub struct BootdiskOptionsView { } impl BootdiskOptionsView { - pub fn new(siv: &mut Cursive, disks: &[Disk], options: &BootdiskOptions) -> Self { + pub fn new(siv: &mut Cursive, runinfo: &RuntimeInfo, options: &BootdiskOptions) -> Self { let bootdisk_form = FormView::new() .child( "Target harddisk", SelectView::new() .popup() - .with_all(disks.iter().map(|d| (d.to_string(), d.clone()))), + .with_all(runinfo.disks.iter().map(|d| (d.to_string(), d.clone()))), ) .with_name("bootdisk-options-target-disk"); @@ -52,11 +56,11 @@ impl BootdiskOptionsView { let advanced_button = LinearLayout::horizontal() .child(DummyView.full_width()) .child(Button::new("Advanced options", { - let disks = disks.to_owned(); + let runinfo = runinfo.clone(); let options = advanced_options.clone(); move |siv| { siv.add_layer(advanced_options_view( - &disks, + &runinfo, options.clone(), product_conf.clone(), )); @@ -109,7 +113,7 @@ struct AdvancedBootdiskOptionsView { } impl AdvancedBootdiskOptionsView { - fn new(disks: &[Disk], options: &BootdiskOptions, product_conf: ProductConfig) -> Self { + fn new(runinfo: &RuntimeInfo, options: &BootdiskOptions, product_conf: ProductConfig) -> Self { let filter_btrfs = |fstype: &&FsType| -> bool { product_conf.enable_btrfs || !fstype.is_btrfs() }; @@ -128,10 +132,7 @@ impl AdvancedBootdiskOptionsView { .position(|t| *t == options.fstype) .unwrap_or_default(), ) - .on_submit({ - let disks = disks.to_owned(); - move |siv, fstype| Self::fstype_on_submit(siv, &disks, fstype) - }); + .on_submit(Self::fstype_on_submit); let mut view = LinearLayout::vertical() .child(DummyView.full_width()) @@ -143,17 +144,17 @@ impl AdvancedBootdiskOptionsView { view.add_child(LvmBootdiskOptionsView::new(lvm, &product_conf)) } AdvancedBootdiskOptions::Zfs(zfs) => { - view.add_child(ZfsBootdiskOptionsView::new(disks, zfs)) + view.add_child(ZfsBootdiskOptionsView::new(runinfo, zfs, &product_conf)) } AdvancedBootdiskOptions::Btrfs(btrfs) => { - view.add_child(BtrfsBootdiskOptionsView::new(disks, btrfs)) + view.add_child(BtrfsBootdiskOptionsView::new(&runinfo.disks, btrfs)) } }; Self { view } } - fn fstype_on_submit(siv: &mut Cursive, disks: &[Disk], fstype: &FsType) { + fn fstype_on_submit(siv: &mut Cursive, fstype: &FsType) { let state = siv.user_data::().unwrap(); let runinfo = state.runtime_info.clone(); let product_conf = state.setup_info.config.clone(); @@ -165,14 +166,14 @@ impl AdvancedBootdiskOptionsView { view.remove_child(3); match fstype { FsType::Ext4 | FsType::Xfs => view.add_child( - LvmBootdiskOptionsView::new_with_defaults(&disks[0], &product_conf), + LvmBootdiskOptionsView::new_with_defaults(&runinfo.disks[0], &product_conf), ), FsType::Zfs(_) => view.add_child(ZfsBootdiskOptionsView::new_with_defaults( &runinfo, &product_conf, )), FsType::Btrfs(_) => { - view.add_child(BtrfsBootdiskOptionsView::new_with_defaults(disks)) + view.add_child(BtrfsBootdiskOptionsView::new_with_defaults(&runinfo.disks)) } } } @@ -186,7 +187,7 @@ impl AdvancedBootdiskOptionsView { 0, SelectView::new() .popup() - .with_all(disks.iter().map(|d| (d.to_string(), d.clone()))), + .with_all(runinfo.disks.iter().map(|d| (d.to_string(), d.clone()))), ); } other => view.replace_child(0, TextView::new(other.to_string())), @@ -516,7 +517,13 @@ struct ZfsBootdiskOptionsView { impl ZfsBootdiskOptionsView { // TODO: Re-apply previous disk selection from `options` correctly - fn new(disks: &[Disk], options: &ZfsBootdiskOptions) -> Self { + fn new( + runinfo: &RuntimeInfo, + options: &ZfsBootdiskOptions, + product_conf: &ProductConfig, + ) -> Self { + let is_pve = product_conf.product == ProxmoxProduct::PVE; + let inner = FormView::new() .child("ashift", IntegerEditView::new().content(options.ashift)) .child( @@ -544,9 +551,16 @@ impl ZfsBootdiskOptionsView { ), ) .child("copies", IntegerEditView::new().content(options.copies)) + .child_conditional( + is_pve, + "ARC max size", + IntegerEditView::new_with_suffix("MiB") + .max_value(runinfo.total_memory) + .content(options.arc_max), + ) .child("hdsize", DiskSizeEditView::new().content(options.disk_size)); - let view = MultiDiskOptionsView::new(disks, &options.selected_disks, inner) + let view = MultiDiskOptionsView::new(&runinfo.disks, &options.selected_disks, inner) .top_panel(TextView::new( "ZFS is not compatible with hardware RAID controllers, for details see the documentation." ).center()); @@ -556,20 +570,30 @@ impl ZfsBootdiskOptionsView { fn new_with_defaults(runinfo: &RuntimeInfo, product_conf: &ProductConfig) -> Self { Self::new( - &runinfo.disks, + runinfo, &ZfsBootdiskOptions::defaults_from(runinfo, product_conf), + product_conf, ) } 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::(4)?; + 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 + }; Some(( disks, @@ -578,7 +602,7 @@ impl ZfsBootdiskOptionsView { compress, checksum, copies, - arc_max: 0, // use built-in ZFS default value + arc_max, disk_size, selected_disks, }, @@ -591,12 +615,12 @@ impl ViewWrapper for ZfsBootdiskOptionsView { } fn advanced_options_view( - disks: &[Disk], + runinfo: &RuntimeInfo, options: Rc>, product_conf: ProductConfig, ) -> impl View { Dialog::around(AdvancedBootdiskOptionsView::new( - disks, + runinfo, &(*options).borrow(), product_conf, )) diff --git a/proxmox-tui-installer/src/views/mod.rs b/proxmox-tui-installer/src/views/mod.rs index 1777c09..4d27532 100644 --- a/proxmox-tui-installer/src/views/mod.rs +++ b/proxmox-tui-installer/src/views/mod.rs @@ -43,7 +43,6 @@ impl NumericEditView { /// /// # Arguments /// * `suffix` - Content for the label to the right of it. - #[allow(unused)] pub fn new_with_suffix(suffix: &str) -> Self { let view = LinearLayout::horizontal() .child(EditView::new().content("0").full_width()) -- 2.42.0