Virtual devices such as e.g. bridges, bonds etc. otherwise also show up, where name pinning does not make sense.
The `pinned_id` property has been made optional, thus adapt to that. Not really an issue during bare-metal installations, as there aren't any bridges/bonds set up yet - no changes there. But it's a nice future-proofing and improves developing/testing experience, as such interfaces are now suppressed). Signed-off-by: Christoph Heiss <[email protected]> --- proxmox-installer-common/src/options.rs | 31 ++++++++++++-------- proxmox-installer-common/src/setup.rs | 33 +++++++++++++--------- proxmox-tui-installer/src/views/network.rs | 33 +++++++++++++++------- 3 files changed, 61 insertions(+), 36 deletions(-) diff --git a/proxmox-installer-common/src/options.rs b/proxmox-installer-common/src/options.rs index a07fe58..8f3b3aa 100644 --- a/proxmox-installer-common/src/options.rs +++ b/proxmox-installer-common/src/options.rs @@ -595,8 +595,10 @@ impl NetworkOptions { { // we got some ipv4 connectivity, so use that - if let Some(opts) = pinning_opts { - this.ifname.clone_from(&iface.to_pinned(opts).name); + if let Some(opts) = pinning_opts + && let Some(pinned) = iface.to_pinned(opts) + { + this.ifname.clone_from(&pinned.name); } else { this.ifname.clone_from(&iface.name); } @@ -609,8 +611,10 @@ impl NetworkOptions { && let Some(addr) = iface.addresses.iter().find(|addr| addr.is_ipv6()) { // no ipv4, but ipv6 connectivity - if let Some(opts) = pinning_opts { - this.ifname.clone_from(&iface.to_pinned(opts).name); + if let Some(opts) = pinning_opts + && let Some(pinned) = iface.to_pinned(opts) + { + this.ifname.clone_from(&pinned.name); } else { this.ifname.clone_from(&iface.name); } @@ -627,19 +631,22 @@ impl NetworkOptions { if this.ifname.is_empty() && let Some(iface) = network.interfaces.values().min_by_key(|v| v.index) { - if let Some(opts) = pinning_opts { - this.ifname.clone_from(&iface.to_pinned(opts).name); + if let Some(opts) = pinning_opts + && let Some(pinned) = iface.to_pinned(opts) + { + this.ifname.clone_from(&pinned.name); } else { this.ifname.clone_from(&iface.name); } } if let Some(ref mut opts) = this.pinning_opts { - // Ensure that all unique interfaces indeed have an entry in the map, - // as required by the low-level installer + // Ensure that all unique, pinnable interfaces indeed have an entry in the map, as + // required by the low-level installer for iface in network.interfaces.values() { - let pinned_name = iface.to_pinned(opts).name; - opts.mapping.entry(iface.mac.clone()).or_insert(pinned_name); + if let Some(pinned) = iface.to_pinned(opts) { + opts.mapping.entry(iface.mac.clone()).or_insert(pinned.name); + } } } @@ -767,7 +774,7 @@ mod tests { Interface { name: "eth0".to_owned(), index: 0, - pinned_id: "0".to_owned(), + pinned_id: Some("0".to_owned()), state: InterfaceState::Up, driver: "dummy".to_owned(), mac: "01:23:45:67:89:ab".to_owned(), @@ -901,7 +908,7 @@ mod tests { Interface { name: "eth0".to_owned(), index: 0, - pinned_id: "0".to_owned(), + pinned_id: Some("0".to_owned()), state: InterfaceState::Up, driver: "dummy".to_owned(), mac: "01:23:45:67:89:ab".to_owned(), diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs index c93ee30..35949f0 100644 --- a/proxmox-installer-common/src/setup.rs +++ b/proxmox-installer-common/src/setup.rs @@ -465,8 +465,9 @@ pub struct Interface { pub index: usize, - /// Sequential interface ID for pinning interface names. - pub pinned_id: String, + /// Sequential interface ID for pinning interface names. If unset, the + /// interface name cannot/should not be pinned due to being e.g. a non-physical link. + pub pinned_id: Option<String>, pub mac: String, @@ -492,19 +493,23 @@ impl Interface { ) } - pub fn to_pinned(&self, options: &NetworkInterfacePinningOptions) -> Self { - let mut this = self.clone(); - this.name = options - .mapping - .get(&this.mac) - .unwrap_or(&format!( - "{}{}", - NetworkInterfacePinningOptions::DEFAULT_PREFIX, - this.pinned_id - )) - .clone(); + pub fn to_pinned(&self, options: &NetworkInterfacePinningOptions) -> Option<Self> { + if let Some(pinned_id) = &self.pinned_id { + let mut this = self.clone(); + this.name = options + .mapping + .get(&this.mac) + .unwrap_or(&format!( + "{}{}", + NetworkInterfacePinningOptions::DEFAULT_PREFIX, + pinned_id + )) + .clone(); - this + Some(this) + } else { + None + } } } diff --git a/proxmox-tui-installer/src/views/network.rs b/proxmox-tui-installer/src/views/network.rs index 4388c7d..970c353 100644 --- a/proxmox-tui-installer/src/views/network.rs +++ b/proxmox-tui-installer/src/views/network.rs @@ -189,9 +189,12 @@ impl NetworkOptionsView { .map(|opt| opt.pinning_enabled.then_some(opt.pinning_options.clone())) .map_err(|err| err.to_string())?; - let ifname = match &pinning_opts { - Some(opts) => iface.to_pinned(opts).name, - None => iface.name, + let ifname = if let Some(opts) = &pinning_opts + && let Some(pinned) = iface.to_pinned(opts) + { + pinned.name + } else { + iface.name }; if address.addr().is_ipv4() != gateway.is_ipv4() { @@ -291,13 +294,13 @@ impl NetworkOptionsView { let ifnames = ifaces .iter() .map(|iface| { - let iface = if options.pinning_enabled { - &iface.to_pinned(&options.pinning_options) + if options.pinning_enabled + && let Some(pinned) = iface.to_pinned(&options.pinning_options) + { + (pinned.render(), pinned.clone()) } else { - iface - }; - - (iface.render(), iface.clone()) + (iface.render(), (*iface).clone()) + } }) .collect::<Vec<(String, Interface)>>(); @@ -337,6 +340,11 @@ impl InterfacePinningOptionsView { fn new(interfaces: &[&Interface], 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 mut form = FormView::<String>::new(); for iface in interfaces { @@ -349,7 +357,12 @@ impl InterfacePinningOptionsView { .child(DummyView.full_width()) // right align below form elements .child( EditView::new() - .content(iface.to_pinned(&options.pinning_options).name) + .content( + iface + .to_pinned(&options.pinning_options) + .expect("always pinnable interface") + .name, + ) .max_content_width(MAX_IFNAME_LEN) .fixed_width(MAX_IFNAME_LEN), ); -- 2.51.0 _______________________________________________ pve-devel mailing list [email protected] https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
