[pve-devel] [PATCH installer 0/4] tui: small fixup round

[pve-devel] [PATCH installer 0/4] tui: small fixup round

[Christoph Heiss]

Christoph Heiss (4):
  tui: do not center EULA text
  tui: preserve autoreboot checkbox state when switching views
  tui: add missing argument for low-level installer test-session
  common: enforce even number of disks for ZFS RAID-10

 proxmox-installer-common/src/disk_checks.rs |  8 ++++++
 proxmox-installer-common/src/setup.rs       |  2 +-
 proxmox-tui-installer/src/main.rs           | 31 +++++++++++----------
 3 files changed, 25 insertions(+), 16 deletions(-)

--
2.42.0

[pve-devel] [PATCH installer 1/4] tui: do not center EULA text

[Christoph Heiss]

Brings it in line with the GUI installer.

Reported-by: Fiona Ebner <f.ebner@proxmox.com>
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 proxmox-tui-installer/src/main.rs | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/proxmox-tui-installer/src/main.rs b/proxmox-tui-installer/src/main.rs
index 4b6b5b2..15340e1 100644
--- a/proxmox-tui-installer/src/main.rs
+++ b/proxmox-tui-installer/src/main.rs
@@ -355,9 +355,9 @@ fn license_dialog(siv: &mut Cursive) -> InstallerView {
             0,
             TextView::new("END USER LICENSE AGREEMENT (EULA)").center(),
         ))
-        .child(Panel::new(ScrollView::new(
-            TextView::new(get_eula(&state.setup_info)).center(),
-        )))
+        .child(Panel::new(ScrollView::new(TextView::new(get_eula(
+            &state.setup_info,
+        )))))
         .child(PaddedView::lrtb(1, 1, 1, 0, bbar));
 
     let _ = inner.set_focus_index(2); // ignore errors
-- 
2.42.0

[pve-devel] [PATCH installer 2/4] tui: preserve autoreboot checkbox state when switching views

[Christoph Heiss]

Instead of reading the checkbox when continuing to the next screen, save
its toggle status to the installer state instead on change.

Reported-by: Fiona Ebner <f.ebner@proxmox.com>
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 proxmox-tui-installer/src/main.rs | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/proxmox-tui-installer/src/main.rs b/proxmox-tui-installer/src/main.rs
index 15340e1..2462a58 100644
--- a/proxmox-tui-installer/src/main.rs
+++ b/proxmox-tui-installer/src/main.rs
@@ -8,7 +8,7 @@ use cursive::{
     view::{Nameable, Offset, Resizable, ViewWrapper},
     views::{
         Button, Checkbox, Dialog, DummyView, EditView, Layer, LinearLayout, PaddedView, Panel,
-        ResizedView, ScrollView, SelectView, StackView, TextView, ViewRef,
+        ResizedView, ScrollView, SelectView, StackView, TextView,
     },
     Cursive, CursiveRunnable, ScreenId, View, XY,
 };
@@ -171,7 +171,7 @@ fn main() {
             timezone: TimezoneOptions::defaults_from(&runtime_info, &locales),
             password: Default::default(),
             network: NetworkOptions::defaults_from(&setup_info, &runtime_info.network),
-            autoreboot: false,
+            autoreboot: true,
         },
         setup_info,
         runtime_info,
@@ -612,6 +612,7 @@ impl TableViewItem for SummaryOption {

 fn summary_dialog(siv: &mut Cursive) -> InstallerView {
     let state = siv.user_data::<InstallerState>().unwrap();
+    let autoreboot = state.options.autoreboot;

     let mut bbar = LinearLayout::horizontal()
         .child(abort_install_button())
@@ -619,20 +620,20 @@ fn summary_dialog(siv: &mut Cursive) -> InstallerView {
         .child(Button::new("Previous", switch_to_prev_screen))
         .child(DummyView)
         .child(Button::new("Install", |siv| {
-            let autoreboot = siv
-                .find_name("reboot-after-install")
-                .map(|v: ViewRef<Checkbox>| v.is_checked())
-                .unwrap_or_default();
-
-            siv.with_user_data(|state: &mut InstallerState| {
-                state.options.autoreboot = autoreboot;
-            });
-
             switch_to_next_screen(siv, InstallerStep::Install, &install_progress_dialog);
         }));

     let _ = bbar.set_focus_index(2); // ignore errors

+    let autoreboot_checkbox =
+        Checkbox::new()
+            .with_checked(autoreboot)
+            .on_change(|siv, autoreboot| {
+                siv.with_user_data(|state: &mut InstallerState| {
+                    state.options.autoreboot = autoreboot;
+                });
+            });
+
     let mut inner = LinearLayout::vertical()
         .child(PaddedView::lrtb(
             0,
@@ -649,7 +650,7 @@ fn summary_dialog(siv: &mut Cursive) -> InstallerView {
         .child(
             LinearLayout::horizontal()
                 .child(DummyView.full_width())
-                .child(Checkbox::new().checked().with_name("reboot-after-install"))
+                .child(autoreboot_checkbox)
                 .child(
                     TextView::new(" Automatically reboot after successful installation").no_wrap(),
                 )
--
2.42.0

[pve-devel] [PATCH installer 3/4] tui: add missing argument for low-level installer test-session

[Christoph Heiss]

This broke running the TUI installer in debug mode, does not effect
release builds in any way.

Fixes: 4b4dfa1 ("low level: testmode: take path to disk image instead of using /dev/null")
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 proxmox-installer-common/src/setup.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs
index 70bdc3c..472e1f2 100644
--- a/proxmox-installer-common/src/setup.rs
+++ b/proxmox-installer-common/src/setup.rs
@@ -373,7 +373,7 @@ pub fn spawn_low_level_installer(test_mode: bool) -> io::Result<process::Child>
     let (path, args, envs): (&str, &[&str], Vec<(&str, &str)>) = if test_mode {
         (
             "./proxmox-low-level-installer",
-            &["-t", "start-session-test"],
+            &["-t", "/dev/null", "start-session-test"],
             vec![("PERL5LIB", ".")],
         )
     } else {
-- 
2.42.0

[pve-devel] [PATCH installer 4/4] common: enforce even number of disks for ZFS RAID-10

[Christoph Heiss]

An uneven number of disks otherwise causes a panic due to an
out-of-bounds array access in the loop below.

Reported-by: Fiona Ebner <f.ebner@proxmox.com>
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 proxmox-installer-common/src/disk_checks.rs | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/proxmox-installer-common/src/disk_checks.rs b/proxmox-installer-common/src/disk_checks.rs
index bcf2e21..7cbdf19 100644
--- a/proxmox-installer-common/src/disk_checks.rs
+++ b/proxmox-installer-common/src/disk_checks.rs
@@ -81,6 +81,14 @@ pub fn check_zfs_raid_config(level: ZfsRaidLevel, disks: &[Disk]) -> Result<(),
         }
         ZfsRaidLevel::Raid10 => {
             check_raid_min_disks(disks, 4)?;
+
+            if disks.len() % 2 != 0 {
+                return Err(format!(
+                    "Needs an even number of disks, currently selected: {}",
+                    disks.len(),
+                ));
+            }
+
             // Pairs need to have the same size
             for i in (0..disks.len()).step_by(2) {
                 check_mirror_size(&disks[i], &disks[i + 1])?;
-- 
2.42.0

Re: [pve-devel] [PATCH installer 4/4] common: enforce even number of disks for ZFS RAID-10

[Christoph Heiss]

Sent bit to quickly and missed the proper prefix; this fixes #5050 [0].

[0] https://bugzilla.proxmox.com/show_bug.cgi?id=5050

On Tue, Nov 21, 2023 at 11:45:51AM +0100, Christoph Heiss wrote:
>
> An uneven number of disks otherwise causes a panic due to an
> out-of-bounds array access in the loop below.
>
> Reported-by: Fiona Ebner <f.ebner@proxmox.com>
> Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
> ---
>  proxmox-installer-common/src/disk_checks.rs | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/proxmox-installer-common/src/disk_checks.rs b/proxmox-installer-common/src/disk_checks.rs
> index bcf2e21..7cbdf19 100644
> --- a/proxmox-installer-common/src/disk_checks.rs
> +++ b/proxmox-installer-common/src/disk_checks.rs
> @@ -81,6 +81,14 @@ pub fn check_zfs_raid_config(level: ZfsRaidLevel, disks: &[Disk]) -> Result<(),
>          }
>          ZfsRaidLevel::Raid10 => {
>              check_raid_min_disks(disks, 4)?;
> +
> +            if disks.len() % 2 != 0 {
> +                return Err(format!(
> +                    "Needs an even number of disks, currently selected: {}",
> +                    disks.len(),
> +                ));
> +            }
> +
>              // Pairs need to have the same size
>              for i in (0..disks.len()).step_by(2) {
>                  check_mirror_size(&disks[i], &disks[i + 1])?;
> --
> 2.42.0
>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>

[pve-devel] applied: [PATCH installer 0/4] tui: small fixup round

[Thomas Lamprecht]

Am 21/11/2023 um 11:45 schrieb Christoph Heiss:
> Christoph Heiss (4):
>   tui: do not center EULA text
>   tui: preserve autoreboot checkbox state when switching views
>   tui: add missing argument for low-level installer test-session
>   common: enforce even number of disks for ZFS RAID-10
> 
>  proxmox-installer-common/src/disk_checks.rs |  8 ++++++
>  proxmox-installer-common/src/setup.rs       |  2 +-
>  proxmox-tui-installer/src/main.rs           | 31 +++++++++++----------
>  3 files changed, 25 insertions(+), 16 deletions(-)
> 
> --
> 2.42.0

applied series, thanks!