Virtual devices such as e.g. bridges, bonds etc. otherwise also show up, where name pinning does not make sense.
By making the `pinned_id` property on our internal network device structure optional, any other code can easily determine whether an interface is pinnable or not. 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/Install.pm | 9 +++------ Proxmox/Install/RunEnv.pm | 12 +++++++++--- Proxmox/Sys/Net.pm | 18 ------------------ proxinstall | 10 ++++++++-- 4 files changed, 20 insertions(+), 29 deletions(-) diff --git a/Proxmox/Install.pm b/Proxmox/Install.pm index 1704ec4..bfc1769 100644 --- a/Proxmox/Install.pm +++ b/Proxmox/Install.pm @@ -1108,15 +1108,12 @@ sub extract_data { mkdir "$targetdir/usr/local/lib/systemd", 0755; mkdir "$targetdir/usr/local/lib/systemd/network", 0755; - my $ip_links = Proxmox::Sys::Net::ip_link_details(); - for my $ifname (sort keys $run_env->{network}->{interfaces}->%*) { - next - if !Proxmox::Sys::Net::ip_link_is_physical($ip_links->{$ifname}) - || Proxmox::Sys::Net::iface_is_vf($ifname); - my $if = $run_env->{network}->{interfaces}->{$ifname}; + # skip unpinnable interfaces, i.e. virtual links + next if !defined($if->{pinned_id}); + if (!defined($netif_pin_map->{ $if->{mac} })) { # each installer frontend must ensure that either # - pinning is disabled by never setting the map or diff --git a/Proxmox/Install/RunEnv.pm b/Proxmox/Install/RunEnv.pm index 40a9621..fd3f74c 100644 --- a/Proxmox/Install/RunEnv.pm +++ b/Proxmox/Install/RunEnv.pm @@ -87,7 +87,7 @@ my sub query_netdevs : prototype() { my $default; # FIXME: not the same as the battle proven way we used in the installer for years? - my $interfaces = fromjs(qx/ip --json address show/); + my $interfaces = fromjs(qx/ip --details --json address show/); my $pinned_counter = 0; for my $if (@$interfaces) { @@ -122,14 +122,20 @@ my sub query_netdevs : prototype() { $ifs->{$name} = { index => $index, name => $name, - pinned_id => "${pinned_counter}", mac => $mac, state => uc($state), driver => $driver, }; $ifs->{$name}->{addresses} = \@valid_addrs if @valid_addrs; - $pinned_counter++; + # only set the `pinned_id` property if the interface actually can be pinned, + # i.e. is a physical link + my $is_pinnable = + Proxmox::Sys::Net::ip_link_is_physical($if) && !Proxmox::Sys::Net::iface_is_vf($name); + if ($is_pinnable) { + $ifs->{$name}->{pinned_id} = "${pinned_counter}"; + $pinned_counter++; + } } return $ifs; diff --git a/Proxmox/Sys/Net.pm b/Proxmox/Sys/Net.pm index 016a7f8..f6a0940 100644 --- a/Proxmox/Sys/Net.pm +++ b/Proxmox/Sys/Net.pm @@ -153,24 +153,6 @@ sub iface_is_vf { return -l "/sys/class/net/$iface_name/device/physfn"; } -# Duplicated from pve-common/src/PVE/Network.pm for now -sub ip_link_details { - my $link_json = ''; - - Proxmox::Sys::Command::run_command( - ['ip', '-details', '-json', 'link', 'show'], - sub { - $link_json .= shift; - return; - }, - ); - - my $links = JSON::decode_json($link_json); - my %ip_links = map { $_->{ifname} => $_ } $links->@*; - - return \%ip_links; -} - # Duplicated from pve-common/src/PVE/Network.pm from now sub ip_link_is_physical { my ($ip_link) = @_; diff --git a/proxinstall b/proxinstall index e3ea22e..e076c25 100755 --- a/proxinstall +++ b/proxinstall @@ -374,9 +374,15 @@ my sub create_network_interface_pin_view { my $inputs = {}; my $row = 0; + for my $ifname (sort keys $interfaces->%*) { my $iface = $interfaces->{$ifname}; + # exclude all non-pinnable interfaces, i.e. non-physical links, which won't have their + # `pinned_id` set. + # It does not make sense to pin those names, and the low-level installer will also skip them + next if !defined($iface->{pinned_id}); + my $name = $mapping->{ $iface->{mac} }; my $label_text = "$iface->{mac} ($ifname, $iface->{driver}, $iface->{state})"; @@ -463,7 +469,7 @@ sub create_ipconf_view { my $symbol = "$iface->{state}" eq "UP" ? "\x{25CF}" : ' '; my $name = - $gtk_state->{network_pinning_enabled} + $gtk_state->{network_pinning_enabled} && defined($mapping->{ $iface->{mac} }) ? $mapping->{ $iface->{mac} } : $iface->{name}; @@ -1920,7 +1926,7 @@ if (!$initial_error && (scalar keys $run_env->{ipconf}->{ifaces}->%* == 0)) { # pre-fill the name mapping before starting my %mapping = map { - $_->{mac} => DEFAULT_PIN_PREFIX . $_->{pinned_id} + defined($_->{pinned_id}) ? ($_->{mac} => DEFAULT_PIN_PREFIX . $_->{pinned_id}) : () } values $run_env->{network}->{interfaces}->%*; Proxmox::Install::Config::set_network_interface_pin_map(\%mapping); } -- 2.51.0 _______________________________________________ pve-devel mailing list [email protected] https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
