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 BD8038900
 for <pve-devel@lists.proxmox.com>; Tue, 22 Aug 2023 12:25:42 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
 by firstgate.proxmox.com (Proxmox) with ESMTP id A1CC3AE37
 for <pve-devel@lists.proxmox.com>; Tue, 22 Aug 2023 12:25:42 +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>; Tue, 22 Aug 2023 12:25:40 +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 C4A42432E0
 for <pve-devel@lists.proxmox.com>; Tue, 22 Aug 2023 12:25:39 +0200 (CEST)
From: Christoph Heiss <c.heiss@proxmox.com>
To: pve-devel@lists.proxmox.com
Date: Tue, 22 Aug 2023 12:24:54 +0200
Message-ID: <20230822102533.295530-7-c.heiss@proxmox.com>
X-Mailer: git-send-email 2.41.0
In-Reply-To: <20230822102533.295530-1-c.heiss@proxmox.com>
References: <20230822102533.295530-1-c.heiss@proxmox.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
X-SPAM-LEVEL: Spam detection results:  0
 AWL -0.039 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 6/6] fix #4829: tui: bootdisk: expose
 new `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 <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: Tue, 22 Aug 2023 10:25:42 -0000

To set the maximum value for arc_max accordingly, simply pass down
`RuntimeInfo` directly instead of the disks array to the views - which
accounts for about half the churn here.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 proxmox-tui-installer/src/main.rs           |  2 +-
 proxmox-tui-installer/src/views/bootdisk.rs | 53 +++++++++++++++------
 2 files changed, 39 insertions(+), 16 deletions(-)

diff --git a/proxmox-tui-installer/src/main.rs b/proxmox-tui-installer/src/main.rs
index 7bfaf9b..384a460 100644
--- a/proxmox-tui-installer/src/main.rs
+++ b/proxmox-tui-installer/src/main.rs
@@ -433,7 +433,7 @@ fn bootdisk_dialog(siv: &mut Cursive) -> InstallerView {

     InstallerView::new(
         &state,
-        BootdiskOptionsView::new(&state.runtime_info.disks, &state.options.bootdisk)
+        BootdiskOptionsView::new(&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 5ea0cee..b736111 100644
--- a/proxmox-tui-installer/src/views/bootdisk.rs
+++ b/proxmox-tui-installer/src/views/bootdisk.rs
@@ -20,19 +20,21 @@ use crate::{
 };
 use crate::{setup::ProxmoxProduct, InstallerState};

+const ZFS_ARC_MIN_SIZE: usize = 64 * 1024 * 1024; // 64 MiB
+
 pub struct BootdiskOptionsView {
     view: LinearLayout,
     advanced_options: Rc<RefCell<BootdiskOptions>>,
 }

 impl BootdiskOptionsView {
-    pub fn new(disks: &[Disk], options: &BootdiskOptions) -> Self {
+    pub fn new(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");

@@ -41,10 +43,10 @@ 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, options.clone()));
+                    siv.add_layer(advanced_options_view(&runinfo, options.clone()));
                 }
             }));

@@ -87,7 +89,7 @@ struct AdvancedBootdiskOptionsView {
 }

 impl AdvancedBootdiskOptionsView {
-    fn new(disks: &[Disk], options: &BootdiskOptions) -> Self {
+    fn new(runinfo: &RuntimeInfo, options: &BootdiskOptions) -> Self {
         let enable_btrfs = crate::setup_info().config.enable_btrfs;

         let filter_btrfs = |fstype: &&FsType| -> bool { enable_btrfs || !fstype.is_btrfs() };
@@ -108,7 +110,7 @@ impl AdvancedBootdiskOptionsView {
                     .unwrap_or_default(),
             )
             .on_submit({
-                let disks = disks.to_owned();
+                let disks = runinfo.disks.to_owned();
                 move |siv, fstype| Self::fstype_on_submit(siv, &disks, fstype)
             });

@@ -120,10 +122,10 @@ impl AdvancedBootdiskOptionsView {
         match &options.advanced {
             AdvancedBootdiskOptions::Lvm(lvm) => view.add_child(LvmBootdiskOptionsView::new(lvm)),
             AdvancedBootdiskOptions::Zfs(zfs) => {
-                view.add_child(ZfsBootdiskOptionsView::new(disks, zfs))
+                view.add_child(ZfsBootdiskOptionsView::new(runinfo, zfs))
             }
             AdvancedBootdiskOptions::Btrfs(btrfs) => {
-                view.add_child(BtrfsBootdiskOptionsView::new(disks, btrfs))
+                view.add_child(BtrfsBootdiskOptionsView::new(&runinfo.disks, btrfs))
             }
         };

@@ -146,7 +148,7 @@ impl AdvancedBootdiskOptionsView {
                         &LvmBootdiskOptions::defaults_from(&disks[0]),
                     )),
                     FsType::Zfs(_) => view.add_child(ZfsBootdiskOptionsView::new(
-                        disks,
+                        &runinfo,
                         &ZfsBootdiskOptions::defaults_from(&runinfo),
                     )),
                     FsType::Btrfs(_) => view.add_child(BtrfsBootdiskOptionsView::new(
@@ -484,7 +486,9 @@ 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) -> Self {
+        let is_pve = crate::setup_info().config.product == ProxmoxProduct::PVE;
+
         let inner = FormView::new()
             .child("ashift", IntegerEditView::new().content(options.ashift))
             .child(
@@ -512,9 +516,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());
@@ -525,12 +536,21 @@ 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, _>(4)?;
+        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)
+        } else {
+            0 // use built-in ZFS default value
+        };

         Some((
             disks,
@@ -539,7 +559,7 @@ impl ZfsBootdiskOptionsView {
                 compress,
                 checksum,
                 copies,
-                arc_max: 0, // use built-in ZFS default value
+                arc_max,
                 disk_size,
                 selected_disks,
             },
@@ -551,9 +571,12 @@ impl ViewWrapper for ZfsBootdiskOptionsView {
     cursive::wrap_impl!(self.view: MultiDiskOptionsView<FormView>);
 }

-fn advanced_options_view(disks: &[Disk], options: Rc<RefCell<BootdiskOptions>>) -> impl View {
+fn advanced_options_view(
+    runinfo: &RuntimeInfo,
+    options: Rc<RefCell<BootdiskOptions>>,
+) -> impl View {
     Dialog::around(AdvancedBootdiskOptionsView::new(
-        disks,
+        runinfo,
         &(*options).borrow(),
     ))
     .title("Advanced bootdisk options")
--
2.41.0