* [pve-devel] [PATCH installer 1/6] sys: net: pinning: make interface name checks stricter
2025-11-13 13:49 [pve-devel] [PATCH installer 0/6] interface name pinning followups Christoph Heiss
@ 2025-11-13 13:49 ` Christoph Heiss
2025-11-13 13:49 ` [pve-devel] [PATCH installer 2/6] common: " Christoph Heiss
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Christoph Heiss @ 2025-11-13 13:49 UTC (permalink / raw)
To: pve-devel
According to our `pve-iface` schema, names must be at least two
characters long and start with a (latin) letter.
Reported-by: Stoiko Ivanov <s.ivanov@proxmox.com>
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Proxmox/Sys/Net.pm | 17 +++++++++++------
proxinstall | 3 +--
test/validate-link-pin-map.pl | 23 +++++++++++++++++++++--
3 files changed, 33 insertions(+), 10 deletions(-)
diff --git a/Proxmox/Sys/Net.pm b/Proxmox/Sys/Net.pm
index 991f723..016a7f8 100644
--- a/Proxmox/Sys/Net.pm
+++ b/Proxmox/Sys/Net.pm
@@ -13,13 +13,16 @@ our @EXPORT_OK = qw(
parse_ip_mask
parse_fqdn
validate_link_pin_map
+ MIN_IFNAME_LEN
MAX_IFNAME_LEN
DEFAULT_PIN_PREFIX
);
-# Maximum length of the (primary) name of a network interface
-# IFNAMSIZ - 1 to account for NUL byte
use constant {
+ # As dictated by the `pve-iface` schema.
+ MIN_IFNAME_LEN => 2,
+ # Maximum length of the (primary) name of a network interface.
+ # IFNAMSIZ - 1 to account for NUL byte
MAX_IFNAME_LEN => 15,
DEFAULT_PIN_PREFIX => 'nic',
};
@@ -338,8 +341,10 @@ sub validate_link_pin_map : prototype($) {
my $reverse_mapping = {};
while (my ($mac, $name) = each %$mapping) {
- if (!defined($name) || $name eq '') {
- die "interface name for '$mac' cannot be empty\n";
+ if (!defined($name) || length($name) < MIN_IFNAME_LEN) {
+ die "interface name for '$mac' must be at least "
+ . MIN_IFNAME_LEN
+ . " characters long\n";
}
if (length($name) > MAX_IFNAME_LEN) {
@@ -353,8 +358,8 @@ sub validate_link_pin_map : prototype($) {
. "name must not be fully numeric\n";
}
- if ($name =~ m/^[0-9]/) {
- die "interface name '$name' for '$mac' is invalid: name must not start with a number\n";
+ if ($name !~ m/^[a-z]/) {
+ die "interface name '$name' for '$mac' is invalid: name must start with a letter\n";
}
if ($name !~ m/^[a-zA-Z_][a-zA-Z0-9_]*$/) {
diff --git a/proxinstall b/proxinstall
index 49dd796..e3ea22e 100755
--- a/proxinstall
+++ b/proxinstall
@@ -37,8 +37,7 @@ use Proxmox::Sys;
use Proxmox::Sys::Block qw(get_cached_disks);
use Proxmox::Sys::Command qw(syscmd);
use Proxmox::Sys::File qw(file_read_all file_write_all);
-use Proxmox::Sys::Net
- qw(parse_ip_address parse_ip_mask validate_link_pin_map MAX_IFNAME_LEN DEFAULT_PIN_PREFIX);
+use Proxmox::Sys::Net qw(parse_ip_address parse_ip_mask validate_link_pin_map DEFAULT_PIN_PREFIX);
use Proxmox::UI;
my $step_number = 0; # Init number for global function list
diff --git a/test/validate-link-pin-map.pl b/test/validate-link-pin-map.pl
index 6386700..37e8387 100755
--- a/test/validate-link-pin-map.pl
+++ b/test/validate-link-pin-map.pl
@@ -8,7 +8,18 @@ use Test::More;
use Proxmox::Sys::Net qw(validate_link_pin_map);
eval { validate_link_pin_map({ 'ab:cd:ef:12:34:56' => '' }) };
-is($@, "interface name for 'ab:cd:ef:12:34:56' cannot be empty\n");
+is(
+ $@,
+ "interface name for 'ab:cd:ef:12:34:56' must be at least 2 characters long\n",
+ "empty name is rejected",
+);
+
+eval { validate_link_pin_map({ 'ab:cd:ef:12:34:56' => 'a' }) };
+is(
+ $@,
+ "interface name for 'ab:cd:ef:12:34:56' must be at least 2 characters long\n",
+ "1 character name is rejected",
+);
eval { validate_link_pin_map({ 'ab:cd:ef:12:34:56' => 'waytoolonginterfacename' }) };
is(
@@ -30,7 +41,15 @@ is(
eval { validate_link_pin_map({ 'ab:cd:ef:12:34:56' => '0nic' }) };
is(
$@,
- "interface name '0nic' for 'ab:cd:ef:12:34:56' is invalid: name must not start with a number\n",
+ "interface name '0nic' for 'ab:cd:ef:12:34:56' is invalid: name must start with a letter\n",
+ "name starting with number is rejected",
+);
+
+eval { validate_link_pin_map({ 'ab:cd:ef:12:34:56' => '_a' }) };
+is(
+ $@,
+ "interface name '_a' for 'ab:cd:ef:12:34:56' is invalid: name must start with a letter\n",
+ "name starting with underscore is rejected",
);
eval { validate_link_pin_map({ 'ab:cd:ef:12:34:56' => '12345' }) };
--
2.51.0
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 8+ messages in thread* [pve-devel] [PATCH installer 2/6] common: pinning: make interface name checks stricter
2025-11-13 13:49 [pve-devel] [PATCH installer 0/6] interface name pinning followups Christoph Heiss
2025-11-13 13:49 ` [pve-devel] [PATCH installer 1/6] sys: net: pinning: make interface name checks stricter Christoph Heiss
@ 2025-11-13 13:49 ` Christoph Heiss
2025-11-13 13:49 ` [pve-devel] [PATCH installer 3/6] test: validate-link-pin-map: give all tests a name Christoph Heiss
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Christoph Heiss @ 2025-11-13 13:49 UTC (permalink / raw)
To: pve-devel
According to our `pve-iface` schema, names must be at least two
characters long and start with a (latin) letter.
Reported-by: Stoiko Ivanov <s.ivanov@proxmox.com>
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
proxmox-installer-common/src/lib.rs | 7 ++--
proxmox-installer-common/src/options.rs | 46 ++++++++++++++++++++-----
2 files changed, 42 insertions(+), 11 deletions(-)
diff --git a/proxmox-installer-common/src/lib.rs b/proxmox-installer-common/src/lib.rs
index a85d5f8..b380f1c 100644
--- a/proxmox-installer-common/src/lib.rs
+++ b/proxmox-installer-common/src/lib.rs
@@ -11,8 +11,11 @@ pub mod http;
pub mod cli;
pub mod net {
- /// Maximum length of the (primary) name of a network interface
- pub const MAX_IFNAME_LEN: usize = 15; // IFNAMSIZ - 1 to account for NUL byte
+ /// As dictated by the `pve-iface` schema.
+ pub const MIN_IFNAME_LEN: usize = 2;
+ /// Maximum length of the (primary) name of a network interface.
+ /// IFNAMSIZ - 1 to account for NUL byte
+ pub const MAX_IFNAME_LEN: usize = 15;
}
pub const RUNTIME_DIR: &str = "/run/proxmox-installer";
diff --git a/proxmox-installer-common/src/options.rs b/proxmox-installer-common/src/options.rs
index 0d72f54..a07fe58 100644
--- a/proxmox-installer-common/src/options.rs
+++ b/proxmox-installer-common/src/options.rs
@@ -8,7 +8,7 @@ use std::sync::OnceLock;
use std::{cmp, fmt};
use crate::disk_checks::check_raid_min_disks;
-use crate::net::MAX_IFNAME_LEN;
+use crate::net::{MAX_IFNAME_LEN, MIN_IFNAME_LEN};
use crate::setup::{LocaleInfo, NetworkInfo, RuntimeInfo, SetupInfo};
use crate::utils::{CidrAddress, Fqdn};
@@ -504,8 +504,10 @@ impl NetworkInterfacePinningOptions {
pub fn verify(&self) -> Result<()> {
let mut reverse_mapping = HashMap::<String, String>::new();
for (mac, name) in self.mapping.iter() {
- if name.is_empty() {
- bail!("interface name for '{mac}' cannot be empty");
+ if name.len() < MIN_IFNAME_LEN {
+ bail!(
+ "interface name for '{mac}' must be at least {MIN_IFNAME_LEN} characters long"
+ );
}
if name.len() > MAX_IFNAME_LEN {
@@ -522,9 +524,9 @@ impl NetworkInterfacePinningOptions {
}
// Mimicking the `pve-iface` schema verification
- if name.starts_with(|c: char| c.is_ascii_digit()) {
+ if !name.starts_with(|c: char| c.is_ascii_alphabetic()) {
bail!(
- "interface name '{name}' for '{mac}' is invalid: name must not start with a number"
+ "interface name '{name}' for '{mac}' is invalid: name must start with a letter"
);
}
@@ -943,7 +945,22 @@ mod tests {
assert!(res.is_err());
assert_eq!(
res.unwrap_err().to_string(),
- "interface name for 'ab:cd:ef:12:34:56' cannot be empty"
+ "interface name for 'ab:cd:ef:12:34:56' must be at least 2 characters long"
+ )
+ }
+
+ #[test]
+ fn network_interface_pinning_options_fail_on_too_short_name() {
+ let mut options = NetworkInterfacePinningOptions::default();
+ options
+ .mapping
+ .insert("ab:cd:ef:12:34:56".to_owned(), "a".to_owned());
+
+ let res = options.verify();
+ assert!(res.is_err());
+ assert_eq!(
+ res.unwrap_err().to_string(),
+ "interface name for 'ab:cd:ef:12:34:56' must be at least 2 characters long"
)
}
@@ -1000,7 +1017,7 @@ mod tests {
}
#[test]
- fn network_interface_pinning_options_fail_on_name_starting_with_number() {
+ fn network_interface_pinning_options_fail_on_nonletter_first_char() {
let mut options = NetworkInterfacePinningOptions::default();
options
.mapping
@@ -1010,8 +1027,19 @@ mod tests {
assert!(res.is_err());
assert_eq!(
res.unwrap_err().to_string(),
- "interface name '0nic' for 'ab:cd:ef:12:34:56' is invalid: name must not start with a number"
- )
+ "interface name '0nic' for 'ab:cd:ef:12:34:56' is invalid: name must start with a letter"
+ );
+
+ options
+ .mapping
+ .insert("ab:cd:ef:12:34:56".to_owned(), "_a".to_owned());
+
+ let res = options.verify();
+ assert!(res.is_err());
+ assert_eq!(
+ res.unwrap_err().to_string(),
+ "interface name '_a' for 'ab:cd:ef:12:34:56' is invalid: name must start with a letter"
+ );
}
#[test]
--
2.51.0
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 8+ messages in thread* [pve-devel] [PATCH installer 3/6] test: validate-link-pin-map: give all tests a name
2025-11-13 13:49 [pve-devel] [PATCH installer 0/6] interface name pinning followups Christoph Heiss
2025-11-13 13:49 ` [pve-devel] [PATCH installer 1/6] sys: net: pinning: make interface name checks stricter Christoph Heiss
2025-11-13 13:49 ` [pve-devel] [PATCH installer 2/6] common: " Christoph Heiss
@ 2025-11-13 13:49 ` Christoph Heiss
2025-11-13 13:49 ` [pve-devel] [PATCH installer 4/6] gui: network: only display pinnable devices in pinning options dialog Christoph Heiss
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Christoph Heiss @ 2025-11-13 13:49 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
test/validate-link-pin-map.pl | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/test/validate-link-pin-map.pl b/test/validate-link-pin-map.pl
index 37e8387..3a95aab 100755
--- a/test/validate-link-pin-map.pl
+++ b/test/validate-link-pin-map.pl
@@ -25,17 +25,21 @@ eval { validate_link_pin_map({ 'ab:cd:ef:12:34:56' => 'waytoolonginterfacename'
is(
$@,
"interface name 'waytoolonginterfacename' for 'ab:cd:ef:12:34:56' cannot be longer than 15 characters\n",
+ "too long name is rejected",
);
eval { validate_link_pin_map({ 'ab:cd:ef:12:34:56' => 'nic0', '12:34:56:ab:cd:ef' => 'nic0' }) };
-like($@, qr/^duplicate interface name mapping 'nic0' for: /);
-like($@, qr/ab:cd:ef:12:34:56/);
-like($@, qr/12:34:56:ab:cd:ef/);
+like(
+ $@, qr/^duplicate interface name mapping 'nic0' for: /, "duplicate assignment is rejected",
+);
+like($@, qr/ab:cd:ef:12:34:56/, "duplicate error message contains expected mac address");
+like($@, qr/12:34:56:ab:cd:ef/, "duplicate error message contains expected mac address");
eval { validate_link_pin_map({ 'ab:cd:ef:12:34:56' => 'nic-' }) };
is(
$@,
"interface name 'nic-' for 'ab:cd:ef:12:34:56' is invalid: name must only consist of alphanumeric characters and underscores\n",
+ "name with dash is rejected",
);
eval { validate_link_pin_map({ 'ab:cd:ef:12:34:56' => '0nic' }) };
@@ -56,6 +60,7 @@ eval { validate_link_pin_map({ 'ab:cd:ef:12:34:56' => '12345' }) };
is(
$@,
"interface name '12345' for 'ab:cd:ef:12:34:56' is invalid: name must not be fully numeric\n",
+ "fully numeric name is rejected",
);
done_testing();
--
2.51.0
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 8+ messages in thread* [pve-devel] [PATCH installer 4/6] gui: network: only display pinnable devices in pinning options dialog
2025-11-13 13:49 [pve-devel] [PATCH installer 0/6] interface name pinning followups Christoph Heiss
` (2 preceding siblings ...)
2025-11-13 13:49 ` [pve-devel] [PATCH installer 3/6] test: validate-link-pin-map: give all tests a name Christoph Heiss
@ 2025-11-13 13:49 ` Christoph Heiss
2025-11-13 13:49 ` [pve-devel] [PATCH installer 5/6] tui: " Christoph Heiss
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Christoph Heiss @ 2025-11-13 13:49 UTC (permalink / raw)
To: pve-devel
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 <c.heiss@proxmox.com>
---
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
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 8+ messages in thread* [pve-devel] [PATCH installer 5/6] tui: network: only display pinnable devices in pinning options dialog
2025-11-13 13:49 [pve-devel] [PATCH installer 0/6] interface name pinning followups Christoph Heiss
` (3 preceding siblings ...)
2025-11-13 13:49 ` [pve-devel] [PATCH installer 4/6] gui: network: only display pinnable devices in pinning options dialog Christoph Heiss
@ 2025-11-13 13:49 ` Christoph Heiss
2025-11-13 13:49 ` [pve-devel] [PATCH installer 6/6] tree-wide: run cargo fmt Christoph Heiss
2025-11-13 20:00 ` [pve-devel] applied: [PATCH installer 0/6] interface name pinning followups Thomas Lamprecht
6 siblings, 0 replies; 8+ messages in thread
From: Christoph Heiss @ 2025-11-13 13:49 UTC (permalink / raw)
To: pve-devel
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 <c.heiss@proxmox.com>
---
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
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 8+ messages in thread* [pve-devel] [PATCH installer 6/6] tree-wide: run cargo fmt
2025-11-13 13:49 [pve-devel] [PATCH installer 0/6] interface name pinning followups Christoph Heiss
` (4 preceding siblings ...)
2025-11-13 13:49 ` [pve-devel] [PATCH installer 5/6] tui: " Christoph Heiss
@ 2025-11-13 13:49 ` Christoph Heiss
2025-11-13 20:00 ` [pve-devel] applied: [PATCH installer 0/6] interface name pinning followups Thomas Lamprecht
6 siblings, 0 replies; 8+ messages in thread
From: Christoph Heiss @ 2025-11-13 13:49 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
.../src/bin/proxmox-auto-installer.rs | 7 ++++---
proxmox-auto-installer/src/utils.rs | 15 +++++++++------
proxmox-fetch-answer/src/fetch_plugins/http.rs | 7 ++++---
.../src/fetch_plugins/partition.rs | 7 ++++---
proxmox-tui-installer/src/main.rs | 7 ++++---
5 files changed, 25 insertions(+), 18 deletions(-)
diff --git a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
index 467ef1b..7614fbb 100644
--- a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
+++ b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
@@ -123,9 +123,10 @@ fn main() -> ExitCode {
};
if answer.global.reboot_on_error
- && let Err(err) = File::create("/run/proxmox-reboot-on-error") {
- error!("failed to create reboot-on-error flag-file: {err}");
- }
+ && let Err(err) = File::create("/run/proxmox-reboot-on-error")
+ {
+ error!("failed to create reboot-on-error flag-file: {err}");
+ }
if answer.global.reboot_mode == RebootMode::PowerOff {
if let Err(err) = File::create("/run/proxmox-poweroff-after-install") {
diff --git a/proxmox-auto-installer/src/utils.rs b/proxmox-auto-installer/src/utils.rs
index f5c3f70..884a08e 100644
--- a/proxmox-auto-installer/src/utils.rs
+++ b/proxmox-auto-installer/src/utils.rs
@@ -426,9 +426,10 @@ pub fn verify_disks_settings(answer: &Answer) -> Result<()> {
}
if let answer::FsOptions::LVM(lvm) = &answer.disks.fs_options
- && let Some((swapsize, hdsize)) = lvm.swapsize.zip(lvm.hdsize) {
- check_swapsize(swapsize, hdsize)?;
- }
+ && let Some((swapsize, hdsize)) = lvm.swapsize.zip(lvm.hdsize)
+ {
+ check_swapsize(swapsize, hdsize)?;
+ }
Ok(())
}
@@ -437,9 +438,11 @@ pub fn verify_first_boot_settings(answer: &Answer) -> Result<()> {
info!("Verifying first boot settings");
if let Some(first_boot) = &answer.first_boot
- && first_boot.source == FirstBootHookSourceMode::FromUrl && first_boot.url.is_none() {
- bail!("first-boot executable source set to URL, but none specified!");
- }
+ && first_boot.source == FirstBootHookSourceMode::FromUrl
+ && first_boot.url.is_none()
+ {
+ bail!("first-boot executable source set to URL, but none specified!");
+ }
Ok(())
}
diff --git a/proxmox-fetch-answer/src/fetch_plugins/http.rs b/proxmox-fetch-answer/src/fetch_plugins/http.rs
index d6331d8..e2fd633 100644
--- a/proxmox-fetch-answer/src/fetch_plugins/http.rs
+++ b/proxmox-fetch-answer/src/fetch_plugins/http.rs
@@ -140,9 +140,10 @@ impl FetchFromHTTP {
info!("Retrieving default search domain.");
for line in read_to_string("/etc/resolv.conf")?.lines() {
if let Some((key, value)) = line.split_once(' ')
- && key == "search" {
- return Ok(value.trim().into());
- }
+ && key == "search"
+ {
+ return Ok(value.trim().into());
+ }
}
bail!("Could not find search domain in resolv.conf.");
}
diff --git a/proxmox-fetch-answer/src/fetch_plugins/partition.rs b/proxmox-fetch-answer/src/fetch_plugins/partition.rs
index f952513..3eac301 100644
--- a/proxmox-fetch-answer/src/fetch_plugins/partition.rs
+++ b/proxmox-fetch-answer/src/fetch_plugins/partition.rs
@@ -123,9 +123,10 @@ fn check_if_mounted(target_path: &str) -> Result<bool> {
let mounts = fs::read_to_string("/proc/mounts")?;
for line in mounts.lines() {
if let Some(mp) = line.split(' ').nth(1)
- && mp == target_path {
- return Ok(true);
- }
+ && mp == target_path
+ {
+ return Ok(true);
+ }
}
Ok(false)
}
diff --git a/proxmox-tui-installer/src/main.rs b/proxmox-tui-installer/src/main.rs
index 05e6a8e..d2fd3d8 100644
--- a/proxmox-tui-installer/src/main.rs
+++ b/proxmox-tui-installer/src/main.rs
@@ -197,9 +197,10 @@ fn installer_setup_late(siv: &mut Cursive) {
if !state.in_test_mode {
let kmap_id = &state.options.timezone.kb_layout;
if let Some(kmap) = state.locales.kmap.get(kmap_id)
- && let Err(err) = system::set_keyboard_layout(kmap) {
- display_setup_warning(siv, &format!("Failed to apply keyboard layout: {err}"));
- }
+ && let Err(err) = system::set_keyboard_layout(kmap)
+ {
+ display_setup_warning(siv, &format!("Failed to apply keyboard layout: {err}"));
+ }
}
if state.runtime_info.total_memory < 1024 {
--
2.51.0
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 8+ messages in thread* [pve-devel] applied: [PATCH installer 0/6] interface name pinning followups
2025-11-13 13:49 [pve-devel] [PATCH installer 0/6] interface name pinning followups Christoph Heiss
` (5 preceding siblings ...)
2025-11-13 13:49 ` [pve-devel] [PATCH installer 6/6] tree-wide: run cargo fmt Christoph Heiss
@ 2025-11-13 20:00 ` Thomas Lamprecht
6 siblings, 0 replies; 8+ messages in thread
From: Thomas Lamprecht @ 2025-11-13 20:00 UTC (permalink / raw)
To: pve-devel, Christoph Heiss
On Thu, 13 Nov 2025 14:49:48 +0100, Christoph Heiss wrote:
> Some small followups for the initial name pinning support [0].
>
> First and foremost makes the interface name checks a bit stricter, to
> comply with our `pve-iface` schema - as reported by Stoiko [1]. Thanks!
>
> Patch #4 & #5 exclude non-physical interfaces from being shown in the
> pinning dialog in the GUI and TUI, respectively. While this does change
> anything for normal bare-metal installation, it is a) a bit of
> future-proofing and b) improves developer/testing experience a bit, as
> these interfaces should never be available for pinning anyway.
>
> [...]
Applied, thanks!
[1/6] sys: net: pinning: make interface name checks stricter
commit: 2033a3576fae1f684024710541508f16b04d72de
[2/6] common: pinning: make interface name checks stricter
commit: aa0ddcdbff34f8bd3d16e017df3ef160e2e1cdd5
[3/6] test: validate-link-pin-map: give all tests a name
commit: 24f69569adcec258edc74190b111e71858e3ca4d
[4/6] gui: network: only display pinnable devices in pinning options dialog
commit: 3520f0dc1941e06dd4b1192007a3e0e97c7c8f0e
[5/6] tui: network: only display pinnable devices in pinning options dialog
commit: 32b2ea3bb230c22008f7f3af6c9ab7c433069561
[6/6] tree-wide: run cargo fmt
commit: d505f71b3c5da129880ca9b15e95e0946d9933cb
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 8+ messages in thread