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 01E48A9C6 for ; Tue, 27 Jun 2023 16:15:53 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id CC81937802 for ; Tue, 27 Jun 2023 16:15:22 +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 ; Tue, 27 Jun 2023 16:15:21 +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 7E28945072 for ; Tue, 27 Jun 2023 16:15:21 +0200 (CEST) From: Stefan Sterz To: pve-devel@lists.proxmox.com Date: Tue, 27 Jun 2023 16:15:07 +0200 Message-Id: <20230627141507.735206-1-s.sterz@proxmox.com> X-Mailer: git-send-email 2.39.2 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.093 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 - 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, options.rs] Subject: [pve-devel] [PATCH installer v2] tui: persist disk selection for zfs and btrfs 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, 27 Jun 2023 14:15:53 -0000 previously the disk selection was reset if the advanced options dialogue was re-opened. this commit adapts the behavior to restore the previous selection. Signed-off-by: Stefan Sterz --- changes since v2 (thanks @Maximiliano Sandoval): * added a comment that the updated `change_default` functions may panic * added a comment and renamed the `get_value` function to reflect the new return type better (to `get_value_and_selection`). proxmox-tui-installer/src/options.rs | 12 +++++- proxmox-tui-installer/src/views/bootdisk.rs | 43 +++++++++++++++------ 2 files changed, 42 insertions(+), 13 deletions(-) diff --git a/proxmox-tui-installer/src/options.rs b/proxmox-tui-installer/src/options.rs index 5f0612e..62804c2 100644 --- a/proxmox-tui-installer/src/options.rs +++ b/proxmox-tui-installer/src/options.rs @@ -118,12 +118,16 @@ impl LvmBootdiskOptions { #[derive(Clone, Debug)] pub struct BtrfsBootdiskOptions { pub disk_size: f64, + pub selected_disks: Vec, } impl BtrfsBootdiskOptions { - pub fn defaults_from(disk: &Disk) -> Self { + /// This panics if the provided slice is empty. + pub fn defaults_from(disks: &[Disk]) -> Self { + let disk = &disks[0]; Self { disk_size: disk.size, + selected_disks: (0..disks.len()).collect(), } } } @@ -191,16 +195,20 @@ pub struct ZfsBootdiskOptions { pub checksum: ZfsChecksumOption, pub copies: usize, pub disk_size: f64, + pub selected_disks: Vec, } impl ZfsBootdiskOptions { - pub fn defaults_from(disk: &Disk) -> Self { + /// This panics if the provided slice is empty. + pub fn defaults_from(disks: &[Disk]) -> Self { + let disk = &disks[0]; Self { ashift: 12, compress: ZfsCompressOption::default(), checksum: ZfsChecksumOption::default(), copies: 1, disk_size: disk.size, + selected_disks: (0..disks.len()).collect(), } } } diff --git a/proxmox-tui-installer/src/views/bootdisk.rs b/proxmox-tui-installer/src/views/bootdisk.rs index 75f70a1..222cab7 100644 --- a/proxmox-tui-installer/src/views/bootdisk.rs +++ b/proxmox-tui-installer/src/views/bootdisk.rs @@ -137,11 +137,11 @@ impl AdvancedBootdiskOptionsView { )), FsType::Zfs(_) => view.add_child(ZfsBootdiskOptionsView::new( disks, - &ZfsBootdiskOptions::defaults_from(&disks[0]), + &ZfsBootdiskOptions::defaults_from(disks), )), FsType::Btrfs(_) => view.add_child(BtrfsBootdiskOptionsView::new( disks, - &BtrfsBootdiskOptions::defaults_from(&disks[0]), + &BtrfsBootdiskOptions::defaults_from(disks), )), } } @@ -274,7 +274,7 @@ struct MultiDiskOptionsView { } impl MultiDiskOptionsView { - fn new(avail_disks: &[Disk], options_view: T) -> Self { + fn new(avail_disks: &[Disk], selected_disks: &Vec, options_view: T) -> Self { let mut selectable_disks = avail_disks .iter() .map(|d| (d.to_string(), Some(d.clone()))) @@ -289,7 +289,7 @@ impl MultiDiskOptionsView { SelectView::new() .popup() .with_all(selectable_disks.clone()) - .selected(i), + .selected(selected_disks[i]), ); } @@ -323,7 +323,13 @@ impl MultiDiskOptionsView { self } - fn get_disks(&mut self) -> Option> { + /// + /// This function returns a tuple of vectors. The first vector contains the currently selected + /// disks in order of their selection slot. Empty slots are filtered out. The second vector + /// contains indices of each slot's selection, which enables us to restore the selection even + /// for empty slots. + /// + fn get_disks_and_selection(&mut self) -> Option<(Vec, Vec)> { let mut disks = vec![]; let view_top_index = usize::from(self.has_top_panel()); @@ -337,6 +343,8 @@ impl MultiDiskOptionsView { .downcast_ref::>()? .get_inner(); + let mut selected_disks = Vec::new(); + for i in 0..disk_form.len() { let disk = disk_form.get_value::>, _>(i)?; @@ -344,9 +352,15 @@ impl MultiDiskOptionsView { if let Some(disk) = disk { disks.push(disk); } + + selected_disks.push( + disk_form + .get_child::>>(i)? + .selected_id()?, + ); } - Some(disks) + Some((disks, selected_disks)) } fn inner_mut(&mut self) -> Option<&mut T> { @@ -378,10 +392,10 @@ struct BtrfsBootdiskOptionsView { } impl BtrfsBootdiskOptionsView { - // TODO: Re-apply previous disk selection from `options` correctly fn new(disks: &[Disk], options: &BtrfsBootdiskOptions) -> Self { let view = MultiDiskOptionsView::new( disks, + &options.selected_disks, FormView::new().child("hdsize", DiskSizeEditView::new().content(options.disk_size)), ) .top_panel(TextView::new("Btrfs integration is a technology preview!").center()); @@ -390,10 +404,16 @@ impl BtrfsBootdiskOptionsView { } fn get_values(&mut self) -> Option<(Vec, BtrfsBootdiskOptions)> { - let disks = self.view.get_disks()?; + let (disks, selected_disks) = self.view.get_disks_and_selection()?; let disk_size = self.view.inner_mut()?.get_value::(0)?; - Some((disks, BtrfsBootdiskOptions { disk_size })) + Some(( + disks, + BtrfsBootdiskOptions { + disk_size, + selected_disks, + }, + )) } } @@ -437,7 +457,7 @@ impl ZfsBootdiskOptionsView { .child("copies", IntegerEditView::new().content(options.copies)) .child("hdsize", DiskSizeEditView::new().content(options.disk_size)); - let view = MultiDiskOptionsView::new(disks, inner) + let view = MultiDiskOptionsView::new(disks, &options.selected_disks, inner) .top_panel(TextView::new( "ZFS is not compatible with hardware RAID controllers, for details see the documentation." ).center()); @@ -446,7 +466,7 @@ impl ZfsBootdiskOptionsView { } fn get_values(&mut self) -> Option<(Vec, ZfsBootdiskOptions)> { - let disks = self.view.get_disks()?; + let (disks, selected_disks) = self.view.get_disks_and_selection()?; let view = self.view.inner_mut()?; let ashift = view.get_value::(0)?; @@ -463,6 +483,7 @@ impl ZfsBootdiskOptionsView { checksum, copies, disk_size, + selected_disks, }, )) } -- 2.39.2