From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id BA8C41FF0B4 for ; Tue, 08 Sep 2026 12:19:19 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id C04B021684; Tue, 08 Sep 2026 12:18:20 +0200 (CEST) From: Christoph Heiss To: pve-devel@lists.proxmox.com Subject: [PATCH installer 14/15] tui: network: factor out name pinning into advanced options dialog Date: Tue, 8 Sep 2026 12:16:28 +0200 Message-ID: <20260908101647.1057780-15-c.heiss@proxmox.com> X-Mailer: git-send-email 2.55.0 In-Reply-To: <20260908101647.1057780-1-c.heiss@proxmox.com> References: <20260908101647.1057780-1-c.heiss@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1788862683640 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.343 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: QPA5KLCT5BS4YAHWNARCKKDA3BDUUETU X-Message-ID-Hash: QPA5KLCT5BS4YAHWNARCKKDA3BDUUETU X-MailFrom: c.heiss@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: This dialog can now be easily extended with additional panels for other, advanced network-related options. Signed-off-by: Christoph Heiss --- proxmox-tui-installer/src/views/network.rs | 272 +++++++++++---------- 1 file changed, 138 insertions(+), 134 deletions(-) diff --git a/proxmox-tui-installer/src/views/network.rs b/proxmox-tui-installer/src/views/network.rs index 6401174..f040881 100644 --- a/proxmox-tui-installer/src/views/network.rs +++ b/proxmox-tui-installer/src/views/network.rs @@ -1,8 +1,9 @@ +use anyhow::{Result, anyhow, bail}; use cursive::{ Cursive, View, view::{Nameable, Resizable, ViewWrapper}, views::{ - Button, Checkbox, Dialog, DummyView, EditView, LinearLayout, NamedView, ResizedView, + Button, Checkbox, Dialog, DummyView, EditView, LinearLayout, NamedView, Panel, ResizedView, ScrollView, SelectView, TextView, }, }; @@ -22,10 +23,10 @@ use proxmox_network_types::{MacAddress, fqdn::Fqdn, ip_address::Cidr}; use super::{CidrAddressEditView, FormView}; struct NetworkViewOptions { - selected_mac: MacAddress, pinning_enabled: bool, - // For UI purposes, we want to always save the mapping, to save the state - // between toggling the checkbox + interfaces: Vec, + // For UI purposes, we want to always save the state for the below options, even if not enabled + // by the user - to preserve the state between toggling the checkbox pinning_options: NetworkInterfacePinningOptions, } @@ -40,32 +41,29 @@ pub struct NetworkOptionsView { } impl NetworkOptionsView { - const PINNING_OPTIONS_BUTTON_NAME: &str = "network-pinning-options-button"; + const FORM_VIEW_NAME: &str = "network-management-form"; const MGMT_IFNAME_SELECTVIEW_NAME: &str = "network-management-ifname-selectview"; + const ADVANCED_OPTIONS_BUTTON_NAME: &str = "network-pinning-options-button"; + + const INTERFACE_PINNING_VIEW_NAME: &str = "network-name-pinning-view"; pub fn new(options: &NetworkOptions, network_info: &NetworkInfo) -> Self { - let mut ifaces = network_info + let mut interfaces = network_info .interfaces .values() - .collect::>(); + .cloned() + .collect::>(); // First, sort interfaces by their link state and then name - ifaces.sort_unstable_by_key(|x| (&x.state, &x.name)); - - let selected_mac = network_info - .interfaces - .get(&options.ifname) - .map(|iface| iface.mac) - .unwrap_or_else(|| ifaces.first().expect("at least one network interface").mac); + interfaces.sort_unstable_by(|a, b| (&a.state, &a.name).cmp(&(&b.state, &b.name))); let options_ref = Arc::new(Mutex::new(NetworkViewOptions { - selected_mac, pinning_enabled: options.pinning_opts.is_some(), + interfaces, pinning_options: options.pinning_opts.clone().unwrap_or_default(), })); - let iface_selection = - Self::build_mgmt_ifname_selectview(ifaces.clone(), options_ref.clone()); + let iface_selection = Self::build_mgmt_ifname_selectview(options_ref.clone(), None); let form = FormView::<()>::new() .child( @@ -89,36 +87,13 @@ impl NetworkOptionsView { EditView::new().content(options.dns_server.to_string()), ); - let pinning_checkbox = LinearLayout::horizontal() - .child(Checkbox::new().checked().on_change({ - let ifaces = ifaces - .iter() - .map(|iface| (*iface).clone()) - .collect::>(); - let options_ref = options_ref.clone(); - move |siv, enable_pinning| { - siv.call_on_name(Self::PINNING_OPTIONS_BUTTON_NAME, { - let options_ref = options_ref.clone(); - move |view: &mut Button| { - view.set_enabled(enable_pinning); - - options_ref.lock().expect("unpoisoned lock").pinning_enabled = - enable_pinning; - } - }); - - Self::refresh_ifname_selectview(siv, &ifaces, options_ref.clone()); - } - })) - .child(TextView::new(" Pin network interface names").no_wrap()) + let advanced_options = LinearLayout::horizontal() .child(DummyView.full_width()) .child( - Button::new("Pinning options", { + Button::new("Advanced options", { let options_ref = options_ref.clone(); - let network_info = network_info.clone(); move |siv| { - let mut view = - Self::custom_name_mapping_view(&network_info, options_ref.clone()); + let mut view = Self::advanced_options_view(options_ref.clone()); // Pre-compute the child's layout, since it might depend on the size. Without this, // the view will be empty until focused. @@ -128,13 +103,13 @@ impl NetworkOptionsView { siv.add_layer(view); } }) - .with_name(Self::PINNING_OPTIONS_BUTTON_NAME), + .with_name(Self::ADVANCED_OPTIONS_BUTTON_NAME), ); let view = LinearLayout::vertical() - .child(form) + .child(form.with_name(Self::FORM_VIEW_NAME)) .child(DummyView.full_width()) - .child(pinning_checkbox); + .child(advanced_options); Self { view, @@ -145,13 +120,10 @@ impl NetworkOptionsView { pub fn get_values(&mut self) -> Result { let form = self .view - .get_child(0) - .and_then(|v| v.downcast_ref::()) - .ok_or("failed to retrieve network options form")?; - - let iface = form - .get_value::>, _>(0) - .ok_or("failed to retrieve management interface name")?; + .get_child_mut(0) + .and_then(|v| v.downcast_mut::>()) + .ok_or("failed to retrieve network options form")? + .get_mut(); let fqdn = form .get_value::(1) @@ -208,83 +180,69 @@ impl NetworkOptionsView { } } - fn custom_name_mapping_view( - network_info: &NetworkInfo, - options_ref: NetworkViewOptionsRef, - ) -> impl View { - const DIALOG_NAME: &str = "network-interface-name-pinning-dialog"; + fn advanced_options_view(options_ref: NetworkViewOptionsRef) -> impl View { + let inner = ScrollView::new( + LinearLayout::vertical().child(Panel::new( + InterfacePinningOptionsView::new(options_ref.clone()) + .with_name(Self::INTERFACE_PINNING_VIEW_NAME), + )), + ); - let mut interfaces = network_info - .interfaces - .values() - .collect::>(); - - interfaces.sort_by(|a, b| (&a.state, &a.name).cmp(&(&b.state, &b.name))); - - Dialog::around(InterfacePinningOptionsView::new( - &interfaces, - options_ref.clone(), - )) - .title("Interface Name Pinning Options") - .button("Ok", { - let interfaces = interfaces - .iter() - .map(|v| (*v).clone()) - .collect::>(); - move |siv| { - let options = siv - .call_on_name(DIALOG_NAME, |view: &mut Dialog| { - view.get_content_mut() - .downcast_mut::() - .map(InterfacePinningOptionsView::get_values) - }) - .flatten(); - - let options = match options { - Some(Ok(options)) => options, - Some(Err(err)) => { - siv.add_layer(Dialog::info(err)); - return; + Dialog::around(inner) + .title("Advanced Network Options") + .button("Ok", { + move |siv| { + match Self::get_view_values(siv) { + Ok(pin_opts) => { + siv.pop_layer(); + let options = &mut options_ref.lock().expect("unpoisoned lock"); + options.pinning_options = pin_opts; + } + Err(err) => { + siv.pop_layer(); + siv.add_layer(Dialog::info(err.to_string())); + return; + } } - None => { - siv.add_layer(Dialog::info( - "Failed to retrieve network interface name pinning options view", - )); - return; - } - }; - siv.pop_layer(); - options_ref.lock().expect("unpoisoned lock").pinning_options = options; - - Self::refresh_ifname_selectview(siv, &interfaces, options_ref.clone()); - } - }) - .with_name(DIALOG_NAME) - .max_size((80, 40)) + Self::refresh_ifname_selectview(siv, options_ref.clone()); + } + }) + .max_size((80, 40)) } - fn refresh_ifname_selectview( - siv: &mut Cursive, - ifaces: &[Interface], - options_ref: NetworkViewOptionsRef, - ) { + fn get_view_values(siv: &mut Cursive) -> Result { + let pin_opts = siv + .call_on_name( + Self::INTERFACE_PINNING_VIEW_NAME, + InterfacePinningOptionsView::get_values, + ) + .unwrap_or_else(|| { + bail!("failed to retrieve network interface name pinning options") + })?; + + Ok(pin_opts) + } + + fn refresh_ifname_selectview(siv: &mut Cursive, options_ref: NetworkViewOptionsRef) { siv.call_on_name( Self::MGMT_IFNAME_SELECTVIEW_NAME, |view: &mut SelectView| { - *view = Self::build_mgmt_ifname_selectview(ifaces.iter().collect(), options_ref); + let selected_mac = view.selection().map(|iface| iface.mac); + *view = Self::build_mgmt_ifname_selectview(options_ref, selected_mac); }, ); } fn build_mgmt_ifname_selectview( - ifaces: Vec<&Interface>, options_ref: NetworkViewOptionsRef, + selected_mac: Option, ) -> SelectView { let options = options_ref.lock().expect("unpoisoned lock"); // Map all interfaces to a list of (human-readable interface name, [Interface]) pairs - let ifnames = ifaces + let ifnames = options + .interfaces .iter() .map(|iface| { if options.pinning_enabled @@ -297,20 +255,20 @@ impl NetworkOptionsView { }) .collect::>(); - let mut view = SelectView::new() - .popup() - .with_all(ifnames.clone()) - .on_submit({ - let options_ref = options_ref.clone(); - move |_, iface| { - options_ref.lock().expect("unpoisoned lock").selected_mac = iface.mac; - } - }); + let mut view = SelectView::new().popup().with_all(ifnames.clone()); + + let selected_mac = selected_mac.unwrap_or_else(|| { + options + .interfaces + .first() + .expect("at least one network interface") + .mac + }); // Finally, (try to) select the current one let selected = view .iter() - .position(|(_label, iface)| iface.mac == options.selected_mac) + .position(|(_label, iface)| iface.mac == selected_mac) .unwrap_or(0); // we sort UP interfaces first, so select the first UP interface // view.set_selection(selected); @@ -324,19 +282,44 @@ impl ViewWrapper for NetworkOptionsView { } struct InterfacePinningOptionsView { - view: ScrollView>>, + view: LinearLayout, } impl InterfacePinningOptionsView { + const ENABLED_CHECKBOX_NAME: &str = "network-interface-name-pinning-enabled"; const FORM_NAME: &str = "network-interface-name-pinning-form"; - fn new(interfaces: &[&Interface], options_ref: NetworkViewOptionsRef) -> Self { + fn new(options_ref: NetworkViewOptionsRef) -> Self { let options = options_ref.lock().expect("unpoisoned lock"); // Filter out all non-physical links, as it does not make sense to pin their names // in this way. // The low-level installer will skip them anyway. - let interfaces = interfaces.iter().filter(|iface| iface.pinned_id.is_some()); + let interfaces = options + .interfaces + .iter() + .filter(|iface| iface.pinned_id.is_some()); + + let mut view = LinearLayout::vertical() + .child( + LinearLayout::horizontal() + .child( + Checkbox::new() + .with_checked(options.pinning_enabled) + .on_change({ + let options_ref = options_ref.clone(); + move |siv, enabled| { + options_ref.lock().expect("unpoisoned lock").pinning_enabled = + enabled; + + Self::toggle_all(siv, enabled); + } + }) + .with_name(Self::ENABLED_CHECKBOX_NAME), + ) + .child(TextView::new(" Pin network interface names").no_wrap()), + ) + .child(DummyView::new()); let mut form = FormView::::new(); @@ -356,7 +339,7 @@ impl InterfacePinningOptionsView { .expect("always pinnable interface") .name, ) - .max_content_width(MAX_IFNAME_LEN) + .with_enabled(options.pinning_enabled) .fixed_width(MAX_IFNAME_LEN), ); @@ -375,13 +358,31 @@ impl InterfacePinningOptionsView { } } - Self { - view: ScrollView::new(form.with_name(Self::FORM_NAME)), - } + view.add_child(form.with_name(Self::FORM_NAME)); + + Self { view } } - fn get_values(&mut self) -> Result { - let form = self.view.get_inner_mut().get_mut(); + fn toggle_all(siv: &mut Cursive, enabled: bool) { + siv.call_on_name(Self::FORM_NAME, |v: &mut FormView| { + v.call_on_childs(&|v: &mut LinearLayout| { + if let Some(v) = v + .get_child_mut(1) + .and_then(|v| v.downcast_mut::>()) + { + v.get_inner_mut().set_enabled(enabled); + } + }); + }); + } + + fn get_values(&mut self) -> Result { + let form = self + .view + .get_child_mut(2) + .and_then(|v| v.downcast_mut::>>()) + .ok_or(anyhow!("failed to retrieve interface pinning form"))? + .get_mut(); let mut mapping = HashMap::new(); @@ -395,18 +396,21 @@ impl InterfacePinningOptionsView { .get_child(1) .and_then(|v| v.downcast_ref::>()) .map(|v| v.get_inner().get_content()) - .ok_or_else(|| format!("failed to retrieve pinning ID for interface {}", mac))?; + .ok_or(anyhow!( + "failed to retrieve pinning ID for interface {}", + mac + ))?; mapping.insert(*mac, (*name).clone()); } let opts = NetworkInterfacePinningOptions { mapping }; - opts.verify().map_err(|err| err.to_string())?; + opts.verify()?; Ok(opts) } } impl ViewWrapper for InterfacePinningOptionsView { - cursive::wrap_impl!(self.view: ScrollView>>); + cursive::wrap_impl!(self.view: LinearLayout); } -- 2.55.0