* [pve-devel] [PATCH installer v3 00/15] support network interface name pinning
@ 2025-11-11 13:59 Christoph Heiss
2025-11-11 13:59 ` [pve-devel] [PATCH installer v3 01/15] test: parse-kernel-cmdline: fix module import statement Christoph Heiss
` (17 more replies)
0 siblings, 18 replies; 21+ messages in thread
From: Christoph Heiss @ 2025-11-11 13:59 UTC (permalink / raw)
To: pve-devel
This series adds support for pinning the names of network interfaces
directly during the installation, for all of GUI, TUI and auto-installer.
There are also some smaller clean-ups and quality-of-life improvements
interspersed - to keep the patches for each part of the installer
together - in the series, which can also be applied
separately/beforehand if wanted.
Tested all combinations, i.e. for each of GUI, TUI and auto-installer,
installed with pinning disabled (checking for regressions) and with
pinning enabled at some custom interface names set.
The auto-installer changes can be tested by specifying e.g.
[network.interface-name-pinning]
enabled = true
[network.interface-name-pinning.mapping]
"ab:cd:ef:12:34:56" = "mgmt"
"12:34:56:ab:cd:ef" = "lan0"
in the answer file.
If pinning is enabled, interfaces which have no explicit mapping
specified in the answer file will be assigned a pinned name based on
their enumeration and the fixed `nic` prefix, i.e. `nic2` for the third
NIC enumerated during boot.
The hardcoded `nic` prefix could potentially be made configurable in the
future, although given that the auto-installer provides information
about all present network interfaces in the answer POST request anyway,
assigning all network interfaces a custom name programmatically is no
big deal.
History
=======
v2: https://lore.proxmox.com/pve-devel/20251030110627.812398-1-c.heiss@proxmox.com/
v1: https://lore.proxmox.com/pve-devel/20251014132207.1171073-1-c.heiss@proxmox.com/
Notable changes v2 -> v3:
* matched up pinning map validation for Rust & Perl (hopefully I
caught all cases this time), also adding unit tests to the latter
* improved above checks for both sides, mostly w.r.t. numeric checks
Notable changes v1 -> v2:
* improved interface name validation according to our `pve-iface` json
schema
* fixed gui error dialog getting wrongly z-ordered
* implemented gtk machinery suggestion by Maximilano
Diffstat
========
Christoph Heiss (14):
test: parse-kernel-cmdline: fix module import statement
run env: network: add kernel driver name to network interface info
common: utils: fix clippy warnings
common: setup: simplify network address list serialization
common: implement support for `network_interface_pin_map` config
auto: add support for pinning network interface names
assistant: verify network settings in `validate-answer` subcommand
post-hook: avoid redundant Option<bool> for (de-)serialization
post-hook: add network interface name and pinning status
tui: views: move network options view to own module
tui: views: form: allow attaching user-defined data to children
tui: add support for pinning network interface names
ui: gtk3: allow passing of dialog parent window
gui: add support for pinning network interface names
Thomas Lamprecht (1):
install: add support for network interface name pinning
Proxmox/Install.pm | 47 +-
Proxmox/Install/Config.pm | 8 +
Proxmox/Install/RunEnv.pm | 11 +
Proxmox/Sys/Net.pm | 116 ++++-
Proxmox/UI.pm | 12 +-
Proxmox/UI/Gtk3.pm | 12 +-
proxinstall | 189 +++++++-
proxmox-auto-install-assistant/src/main.rs | 39 +-
proxmox-auto-installer/src/answer.rs | 63 ++-
.../src/bin/proxmox-auto-installer.rs | 5 +-
proxmox-auto-installer/src/utils.rs | 62 ++-
proxmox-auto-installer/tests/parse-answer.rs | 2 +
.../network_interface_pinning.json | 30 ++
.../network_interface_pinning.toml | 22 +
...n_from_dhcp_no_default_domain.run-env.json | 36 +-
...rface_pinning_overlong_interface_name.json | 3 +
...rface_pinning_overlong_interface_name.toml | 18 +
.../no_fqdn_from_dhcp.run-env.json | 36 +-
.../tests/resources/run-env-info.json | 38 +-
.../src/fetch_plugins/http.rs | 5 +-
.../src/fetch_plugins/partition.rs | 5 +-
proxmox-installer-common/src/lib.rs | 5 +
proxmox-installer-common/src/options.rs | 274 ++++++++++--
proxmox-installer-common/src/setup.rs | 78 +++-
proxmox-installer-common/src/utils.rs | 6 +-
proxmox-post-hook/src/main.rs | 73 ++--
proxmox-tui-installer/src/main.rs | 110 +----
proxmox-tui-installer/src/setup.rs | 3 +
proxmox-tui-installer/src/views/bootdisk.rs | 8 +-
proxmox-tui-installer/src/views/mod.rs | 90 ++--
proxmox-tui-installer/src/views/network.rs | 406 ++++++++++++++++++
test/Makefile | 7 +-
test/parse-kernel-cmdline.pl | 2 +-
test/validate-link-pin-map.pl | 42 ++
34 files changed, 1534 insertions(+), 329 deletions(-)
create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/network_interface_pinning.json
create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/network_interface_pinning.toml
create mode 100644 proxmox-auto-installer/tests/resources/parse_answer_fail/network_interface_pinning_overlong_interface_name.json
create mode 100644 proxmox-auto-installer/tests/resources/parse_answer_fail/network_interface_pinning_overlong_interface_name.toml
create mode 100644 proxmox-tui-installer/src/views/network.rs
create mode 100755 test/validate-link-pin-map.pl
--
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] 21+ messages in thread
* [pve-devel] [PATCH installer v3 01/15] test: parse-kernel-cmdline: fix module import statement
2025-11-11 13:59 [pve-devel] [PATCH installer v3 00/15] support network interface name pinning Christoph Heiss
@ 2025-11-11 13:59 ` Christoph Heiss
2025-11-11 13:59 ` [pve-devel] [PATCH installer v3 02/15] install: add support for network interface name pinning Christoph Heiss
` (16 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Christoph Heiss @ 2025-11-11 13:59 UTC (permalink / raw)
To: pve-devel
The subroutine is called with its fully-qualified path below, so drop
the explicit import - as it otherwise fails to find it.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v2 -> v3:
* no changes
Changes v1 -> v2:
* no changes
test/parse-kernel-cmdline.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/test/parse-kernel-cmdline.pl b/test/parse-kernel-cmdline.pl
index b236734..7c849c5 100755
--- a/test/parse-kernel-cmdline.pl
+++ b/test/parse-kernel-cmdline.pl
@@ -7,7 +7,7 @@ use Test::More;
use Test::MockModule qw(strict);
use Proxmox::Install::RunEnv;
-use Proxmox::Install::Config qw(parse_kernel_cmdline);
+use Proxmox::Install::Config;
my $proxmox_install_runenv = Test::MockModule->new('Proxmox::Install::RunEnv');
my $proxmox_install_isoenv = Test::MockModule->new('Proxmox::Install::ISOEnv');
--
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] 21+ messages in thread
* [pve-devel] [PATCH installer v3 02/15] install: add support for network interface name pinning
2025-11-11 13:59 [pve-devel] [PATCH installer v3 00/15] support network interface name pinning Christoph Heiss
2025-11-11 13:59 ` [pve-devel] [PATCH installer v3 01/15] test: parse-kernel-cmdline: fix module import statement Christoph Heiss
@ 2025-11-11 13:59 ` Christoph Heiss
2025-11-11 13:59 ` [pve-devel] [PATCH installer v3 03/15] run env: network: add kernel driver name to network interface info Christoph Heiss
` (15 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Christoph Heiss @ 2025-11-11 13:59 UTC (permalink / raw)
To: pve-devel
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
Adds the (low-level) part for pinning network interface names during
installation based on their MAC address.
Adds a new option `pin_interface_name_map`, which maps MAC addresses to
their respective new interface name.
Co-authored-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v2 -> v3:
* no changes
Changes v1 -> v2:
* no changes
Proxmox/Install.pm | 47 +++++++++++++++-
Proxmox/Install/Config.pm | 8 +++
Proxmox/Install/RunEnv.pm | 4 ++
Proxmox/Sys/Net.pm | 54 +++++++++++++++++++
...n_from_dhcp_no_default_domain.run-env.json | 27 ++++++----
.../no_fqdn_from_dhcp.run-env.json | 27 ++++++----
.../tests/resources/run-env-info.json | 29 ++++++----
7 files changed, 167 insertions(+), 29 deletions(-)
diff --git a/Proxmox/Install.pm b/Proxmox/Install.pm
index b42d2f0..1704ec4 100644
--- a/Proxmox/Install.pm
+++ b/Proxmox/Install.pm
@@ -1101,6 +1101,49 @@ sub extract_data {
my $cidr = Proxmox::Install::Config::get_cidr();
my $gateway = Proxmox::Install::Config::get_gateway();
+ # configure pinned names for all network interfaces, if enabled
+
+ my $netif_pin_map = Proxmox::Install::Config::get_network_interface_pin_map();
+ if (defined($netif_pin_map)) {
+ 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};
+
+ if (!defined($netif_pin_map->{ $if->{mac} })) {
+ # each installer frontend must ensure that either
+ # - pinning is disabled by never setting the map or
+ # - setting a pinned name for each
+ die "no network interface name set for '$if->{mac}'!";
+ }
+
+ my $pinned_name = $netif_pin_map->{ $if->{mac} };
+ my $link_file_content = Proxmox::Sys::Net::get_pin_link_file_content(
+ $if->{mac}, $pinned_name,
+ );
+
+ file_write_all(
+ "$targetdir/usr/local/lib/systemd/network/50-pmx-${pinned_name}.link",
+ $link_file_content,
+ );
+
+ # ensure that we use the correct interface name when writing out
+ # /etc/network/interfaces below - just in case the interface name
+ # is pinned and get_mngmt_nic() returned the original interface name.
+ # better be safe than sorry
+ if ($ethdev eq $ifname) {
+ $ethdev = $pinned_name;
+ }
+ }
+ }
+
if ($iso_env->{cfg}->{bridged_network}) {
$ifaces .= "iface $ethdev $ntype manual\n";
@@ -1121,7 +1164,9 @@ sub extract_data {
my $ipconf = $run_env->{ipconf};
foreach my $iface (sort keys %{ $ipconf->{ifaces} }) {
- my $name = $ipconf->{ifaces}->{$iface}->{name};
+ my $if = $ipconf->{ifaces}->{$iface};
+ my $name = defined($netif_pin_map) ? $netif_pin_map->{ $if->{mac} } : $if->{name};
+
next if $name eq $ethdev;
$ifaces .= "\niface $name $ntype manual\n";
diff --git a/Proxmox/Install/Config.pm b/Proxmox/Install/Config.pm
index da476da..7b81fcf 100644
--- a/Proxmox/Install/Config.pm
+++ b/Proxmox/Install/Config.pm
@@ -101,6 +101,11 @@ my sub init_cfg {
# network related
mngmt_nic => undef,
+
+ # maps mac address -> custom name
+ # if set to a hash, enables interface name pinning for all interfaces
+ network_interface_pin_map => undef,
+
hostname => undef,
domain => undef,
cidr => undef,
@@ -248,6 +253,9 @@ sub get_root_ssh_keys { return get('root_ssh_keys'); }
sub set_mngmt_nic { set_key('mngmt_nic', $_[0]); }
sub get_mngmt_nic { return get('mngmt_nic'); }
+sub set_network_interface_pin_map { set_key('network_interface_pin_map', $_[0]); }
+sub get_network_interface_pin_map { return get('network_interface_pin_map'); }
+
sub set_hostname { set_key('hostname', $_[0]); }
sub get_hostname { return get('hostname'); }
diff --git a/Proxmox/Install/RunEnv.pm b/Proxmox/Install/RunEnv.pm
index 540bec7..f0fa6ec 100644
--- a/Proxmox/Install/RunEnv.pm
+++ b/Proxmox/Install/RunEnv.pm
@@ -86,6 +86,7 @@ my sub query_netdevs : prototype() {
# 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 $pinned_counter = 0;
for my $if (@$interfaces) {
my ($index, $name, $state, $mac, $addresses) =
$if->@{qw(ifindex ifname operstate address addr_info)};
@@ -115,10 +116,13 @@ my sub query_netdevs : prototype() {
$ifs->{$name} = {
index => $index,
name => $name,
+ pinned_id => "${pinned_counter}",
mac => $mac,
state => uc($state),
};
$ifs->{$name}->{addresses} = \@valid_addrs if @valid_addrs;
+
+ $pinned_counter++;
}
return $ifs;
diff --git a/Proxmox/Sys/Net.pm b/Proxmox/Sys/Net.pm
index 6fe99ec..2183d27 100644
--- a/Proxmox/Sys/Net.pm
+++ b/Proxmox/Sys/Net.pm
@@ -3,7 +3,9 @@ package Proxmox::Sys::Net;
use strict;
use warnings;
+use Proxmox::Sys::Command;
use Proxmox::Sys::Udev;
+use JSON qw();
use base qw(Exporter);
our @EXPORT_OK = qw(parse_ip_address parse_ip_mask parse_fqdn);
@@ -129,9 +131,59 @@ sub parse_ip_mask {
return;
}
+sub iface_is_vf {
+ my ($iface_name) = @_;
+ 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) = @_;
+
+ # ether alone isn't enough, as virtual interfaces can also have link_type
+ # ether
+ return $ip_link->{link_type} eq 'ether'
+ && (!defined($ip_link->{linkinfo}) || !defined($ip_link->{linkinfo}->{info_kind}));
+}
+
+my $LINK_FILE_TEMPLATE = <<EOF;
+# setup by the Proxmox installer.
+[Match]
+MACAddress=%s
+Type=ether
+
+[Link]
+Name=%s
+EOF
+
+sub get_pin_link_file_content {
+ my ($mac, $pin_name) = @_;
+
+ return sprintf($LINK_FILE_TEMPLATE, $mac, $pin_name);
+}
+
sub get_ip_config {
my $ifaces = {};
my $default;
+ my $pinned_counter = 0;
my $links = `ip -o l`;
foreach my $l (split /\n/, $links) {
@@ -144,11 +196,13 @@ sub get_ip_config {
$ifaces->{"$index"} = {
name => $name,
+ pinned_id => "${pinned_counter}",
driver => $driver,
flags => $flags,
state => $state,
mac => $mac,
};
+ $pinned_counter++;
my $addresses = `ip -o a s $name`;
for my $addr_line (split /\n/, $addresses) {
diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/fqdn_from_dhcp_no_default_domain.run-env.json b/proxmox-auto-installer/tests/resources/parse_answer_fail/fqdn_from_dhcp_no_default_domain.run-env.json
index ef14419..e7c2205 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer_fail/fqdn_from_dhcp_no_default_domain.run-env.json
+++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/fqdn_from_dhcp_no_default_domain.run-env.json
@@ -181,55 +181,64 @@
"index": 4,
"mac": "b4:2e:99:ac:ad:b4",
"name": "eno1",
- "state": "UP"
+ "state": "UP",
+ "pinned_id": "0"
},
"eno2": {
"index": 6,
"mac": "b4:2e:99:ac:ad:b5",
"name": "eno2",
- "state": "UP"
+ "state": "UP",
+ "pinned_id": "1"
},
"enp129s0f0np0": {
"index": 7,
"mac": "1c:34:da:5c:5e:24",
"name": "enp129s0f0np0",
- "state": "DOWN"
+ "state": "DOWN",
+ "pinned_id": "2"
},
"enp129s0f1np1": {
"index": 8,
"mac": "1c:34:da:5c:5e:25",
"name": "enp129s0f1np1",
- "state": "DOWN"
+ "state": "DOWN",
+ "pinned_id": "3"
},
"enp193s0f0np0": {
"index": 9,
"mac": "24:8a:07:1e:05:bc",
"name": "enp193s0f0np0",
- "state": "UP"
+ "state": "UP",
+ "pinned_id": "4"
},
"enp193s0f1np1": {
"index": 10,
"mac": "24:8a:07:1e:05:bd",
"name": "enp193s0f1np1",
- "state": "DOWN"
+ "state": "DOWN",
+ "pinned_id": "5"
},
"enp65s0f0": {
"index": 2,
"mac": "a0:36:9f:0a:b3:82",
"name": "enp65s0f0",
- "state": "DOWN"
+ "state": "DOWN",
+ "pinned_id": "6"
},
"enp65s0f1": {
"index": 3,
"mac": "a0:36:9f:0a:b3:83",
"name": "enp65s0f1",
- "state": "DOWN"
+ "state": "DOWN",
+ "pinned_id": "7"
},
"enx5a4732ddc747": {
"index": 5,
"mac": "5a:47:32:dd:c7:47",
"name": "enx5a4732ddc747",
- "state": "UNKNOWN"
+ "state": "UNKNOWN",
+ "pinned_id": "8"
}
},
"routes": {
diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/no_fqdn_from_dhcp.run-env.json b/proxmox-auto-installer/tests/resources/parse_answer_fail/no_fqdn_from_dhcp.run-env.json
index e3cea3e..47d7bde 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer_fail/no_fqdn_from_dhcp.run-env.json
+++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/no_fqdn_from_dhcp.run-env.json
@@ -180,55 +180,64 @@
"index": 4,
"mac": "b4:2e:99:ac:ad:b4",
"name": "eno1",
- "state": "UP"
+ "state": "UP",
+ "pinned_id": "0"
},
"eno2": {
"index": 6,
"mac": "b4:2e:99:ac:ad:b5",
"name": "eno2",
- "state": "UP"
+ "state": "UP",
+ "pinned_id": "1"
},
"enp129s0f0np0": {
"index": 7,
"mac": "1c:34:da:5c:5e:24",
"name": "enp129s0f0np0",
- "state": "DOWN"
+ "state": "DOWN",
+ "pinned_id": "2"
},
"enp129s0f1np1": {
"index": 8,
"mac": "1c:34:da:5c:5e:25",
"name": "enp129s0f1np1",
- "state": "DOWN"
+ "state": "DOWN",
+ "pinned_id": "3"
},
"enp193s0f0np0": {
"index": 9,
"mac": "24:8a:07:1e:05:bc",
"name": "enp193s0f0np0",
- "state": "UP"
+ "state": "UP",
+ "pinned_id": "4"
},
"enp193s0f1np1": {
"index": 10,
"mac": "24:8a:07:1e:05:bd",
"name": "enp193s0f1np1",
- "state": "DOWN"
+ "state": "DOWN",
+ "pinned_id": "5"
},
"enp65s0f0": {
"index": 2,
"mac": "a0:36:9f:0a:b3:82",
"name": "enp65s0f0",
- "state": "DOWN"
+ "state": "DOWN",
+ "pinned_id": "6"
},
"enp65s0f1": {
"index": 3,
"mac": "a0:36:9f:0a:b3:83",
"name": "enp65s0f1",
- "state": "DOWN"
+ "state": "DOWN",
+ "pinned_id": "7"
},
"enx5a4732ddc747": {
"index": 5,
"mac": "5a:47:32:dd:c7:47",
"name": "enx5a4732ddc747",
- "state": "UNKNOWN"
+ "state": "UNKNOWN",
+ "pinned_id": "8"
}
},
"routes": {
diff --git a/proxmox-auto-installer/tests/resources/run-env-info.json b/proxmox-auto-installer/tests/resources/run-env-info.json
index 5a8d80a..5291721 100644
--- a/proxmox-auto-installer/tests/resources/run-env-info.json
+++ b/proxmox-auto-installer/tests/resources/run-env-info.json
@@ -181,55 +181,64 @@
"index": 4,
"mac": "b4:2e:99:ac:ad:b4",
"name": "eno1",
- "state": "UP"
+ "state": "UP",
+ "pinned_id": "0"
},
"eno2": {
"index": 6,
"mac": "b4:2e:99:ac:ad:b5",
"name": "eno2",
- "state": "UP"
+ "state": "UP",
+ "pinned_id": "1"
},
"enp129s0f0np0": {
"index": 7,
"mac": "1c:34:da:5c:5e:24",
"name": "enp129s0f0np0",
- "state": "DOWN"
+ "state": "DOWN",
+ "pinned_id": "2"
},
"enp129s0f1np1": {
"index": 8,
"mac": "1c:34:da:5c:5e:25",
"name": "enp129s0f1np1",
- "state": "DOWN"
+ "state": "DOWN",
+ "pinned_id": "3"
},
"enp193s0f0np0": {
"index": 9,
"mac": "24:8a:07:1e:05:bc",
"name": "enp193s0f0np0",
- "state": "UP"
+ "state": "UP",
+ "pinned_id": "4"
},
"enp193s0f1np1": {
"index": 10,
"mac": "24:8a:07:1e:05:bd",
"name": "enp193s0f1np1",
- "state": "DOWN"
+ "state": "DOWN",
+ "pinned_id": "5"
},
"enp65s0f0": {
"index": 2,
"mac": "a0:36:9f:0a:b3:82",
"name": "enp65s0f0",
- "state": "DOWN"
+ "state": "DOWN",
+ "pinned_id": "6"
},
- "en p65s0f1": {
+ "enp65s0f1": {
"index": 3,
"mac": "a0:36:9f:0a:b3:83",
"name": "enp65s0f1",
- "state": "DOWN"
+ "state": "DOWN",
+ "pinned_id": "7"
},
"enx5a4732ddc747": {
"index": 5,
"mac": "5a:47:32:dd:c7:47",
"name": "enx5a4732ddc747",
- "state": "UNKNOWN"
+ "state": "UNKNOWN",
+ "pinned_id": "8"
}
},
"routes": {
--
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] 21+ messages in thread
* [pve-devel] [PATCH installer v3 03/15] run env: network: add kernel driver name to network interface info
2025-11-11 13:59 [pve-devel] [PATCH installer v3 00/15] support network interface name pinning Christoph Heiss
2025-11-11 13:59 ` [pve-devel] [PATCH installer v3 01/15] test: parse-kernel-cmdline: fix module import statement Christoph Heiss
2025-11-11 13:59 ` [pve-devel] [PATCH installer v3 02/15] install: add support for network interface name pinning Christoph Heiss
@ 2025-11-11 13:59 ` Christoph Heiss
2025-11-11 13:59 ` [pve-devel] [PATCH installer v3 04/15] common: utils: fix clippy warnings Christoph Heiss
` (14 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Christoph Heiss @ 2025-11-11 13:59 UTC (permalink / raw)
To: pve-devel
This information is already available in the GUI, this makes it possible
to show it in the TUI too.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v2 -> v3:
* no changes
Changes v1 -> v2:
* no changes
Proxmox/Install/RunEnv.pm | 7 +++++++
.../fqdn_from_dhcp_no_default_domain.run-env.json | 9 +++++++++
.../parse_answer_fail/no_fqdn_from_dhcp.run-env.json | 9 +++++++++
proxmox-auto-installer/tests/resources/run-env-info.json | 9 +++++++++
proxmox-installer-common/src/options.rs | 2 ++
proxmox-installer-common/src/setup.rs | 3 +++
6 files changed, 39 insertions(+)
diff --git a/Proxmox/Install/RunEnv.pm b/Proxmox/Install/RunEnv.pm
index f0fa6ec..40a9621 100644
--- a/Proxmox/Install/RunEnv.pm
+++ b/Proxmox/Install/RunEnv.pm
@@ -72,6 +72,9 @@ sub query_cpu_info : prototype() {
# mac => <mac address>,
# index => <index>,
# name => <ifname>,
+# state => <UP|DOWN|LOWERLAYERDOWN|DORMANT|TESTING|NOTPRESENT|UNKNOWN>,
+# pinned_id => <sequential numerical ID>,
+# driver => <driver name>,
# addresses => [
# family => <inet|inet6>,
# address => <mac address>,
@@ -113,12 +116,16 @@ my sub query_netdevs : prototype() {
}
}
+ my $driver = readlink "/sys/class/net/$name/device/driver" || 'unknown';
+ $driver =~ s!^.*/!!;
+
$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;
diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/fqdn_from_dhcp_no_default_domain.run-env.json b/proxmox-auto-installer/tests/resources/parse_answer_fail/fqdn_from_dhcp_no_default_domain.run-env.json
index e7c2205..4c41c13 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer_fail/fqdn_from_dhcp_no_default_domain.run-env.json
+++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/fqdn_from_dhcp_no_default_domain.run-env.json
@@ -182,6 +182,7 @@
"mac": "b4:2e:99:ac:ad:b4",
"name": "eno1",
"state": "UP",
+ "driver": "igb",
"pinned_id": "0"
},
"eno2": {
@@ -189,6 +190,7 @@
"mac": "b4:2e:99:ac:ad:b5",
"name": "eno2",
"state": "UP",
+ "driver": "igb",
"pinned_id": "1"
},
"enp129s0f0np0": {
@@ -196,6 +198,7 @@
"mac": "1c:34:da:5c:5e:24",
"name": "enp129s0f0np0",
"state": "DOWN",
+ "driver": "mlx5_core",
"pinned_id": "2"
},
"enp129s0f1np1": {
@@ -203,6 +206,7 @@
"mac": "1c:34:da:5c:5e:25",
"name": "enp129s0f1np1",
"state": "DOWN",
+ "driver": "mlx5_core",
"pinned_id": "3"
},
"enp193s0f0np0": {
@@ -210,6 +214,7 @@
"mac": "24:8a:07:1e:05:bc",
"name": "enp193s0f0np0",
"state": "UP",
+ "driver": "mlx5_core",
"pinned_id": "4"
},
"enp193s0f1np1": {
@@ -217,6 +222,7 @@
"mac": "24:8a:07:1e:05:bd",
"name": "enp193s0f1np1",
"state": "DOWN",
+ "driver": "mlx5_core",
"pinned_id": "5"
},
"enp65s0f0": {
@@ -224,6 +230,7 @@
"mac": "a0:36:9f:0a:b3:82",
"name": "enp65s0f0",
"state": "DOWN",
+ "driver": "igb",
"pinned_id": "6"
},
"enp65s0f1": {
@@ -231,6 +238,7 @@
"mac": "a0:36:9f:0a:b3:83",
"name": "enp65s0f1",
"state": "DOWN",
+ "driver": "igb",
"pinned_id": "7"
},
"enx5a4732ddc747": {
@@ -238,6 +246,7 @@
"mac": "5a:47:32:dd:c7:47",
"name": "enx5a4732ddc747",
"state": "UNKNOWN",
+ "driver": "cdc_ether",
"pinned_id": "8"
}
},
diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/no_fqdn_from_dhcp.run-env.json b/proxmox-auto-installer/tests/resources/parse_answer_fail/no_fqdn_from_dhcp.run-env.json
index 47d7bde..fa4e834 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer_fail/no_fqdn_from_dhcp.run-env.json
+++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/no_fqdn_from_dhcp.run-env.json
@@ -181,6 +181,7 @@
"mac": "b4:2e:99:ac:ad:b4",
"name": "eno1",
"state": "UP",
+ "driver": "igb",
"pinned_id": "0"
},
"eno2": {
@@ -188,6 +189,7 @@
"mac": "b4:2e:99:ac:ad:b5",
"name": "eno2",
"state": "UP",
+ "driver": "igb",
"pinned_id": "1"
},
"enp129s0f0np0": {
@@ -195,6 +197,7 @@
"mac": "1c:34:da:5c:5e:24",
"name": "enp129s0f0np0",
"state": "DOWN",
+ "driver": "mlx5_core",
"pinned_id": "2"
},
"enp129s0f1np1": {
@@ -202,6 +205,7 @@
"mac": "1c:34:da:5c:5e:25",
"name": "enp129s0f1np1",
"state": "DOWN",
+ "driver": "mlx5_core",
"pinned_id": "3"
},
"enp193s0f0np0": {
@@ -209,6 +213,7 @@
"mac": "24:8a:07:1e:05:bc",
"name": "enp193s0f0np0",
"state": "UP",
+ "driver": "mlx5_core",
"pinned_id": "4"
},
"enp193s0f1np1": {
@@ -216,6 +221,7 @@
"mac": "24:8a:07:1e:05:bd",
"name": "enp193s0f1np1",
"state": "DOWN",
+ "driver": "mlx5_core",
"pinned_id": "5"
},
"enp65s0f0": {
@@ -223,6 +229,7 @@
"mac": "a0:36:9f:0a:b3:82",
"name": "enp65s0f0",
"state": "DOWN",
+ "driver": "igb",
"pinned_id": "6"
},
"enp65s0f1": {
@@ -230,6 +237,7 @@
"mac": "a0:36:9f:0a:b3:83",
"name": "enp65s0f1",
"state": "DOWN",
+ "driver": "igb",
"pinned_id": "7"
},
"enx5a4732ddc747": {
@@ -237,6 +245,7 @@
"mac": "5a:47:32:dd:c7:47",
"name": "enx5a4732ddc747",
"state": "UNKNOWN",
+ "driver": "cdc_ether",
"pinned_id": "8"
}
},
diff --git a/proxmox-auto-installer/tests/resources/run-env-info.json b/proxmox-auto-installer/tests/resources/run-env-info.json
index 5291721..add5f2a 100644
--- a/proxmox-auto-installer/tests/resources/run-env-info.json
+++ b/proxmox-auto-installer/tests/resources/run-env-info.json
@@ -182,6 +182,7 @@
"mac": "b4:2e:99:ac:ad:b4",
"name": "eno1",
"state": "UP",
+ "driver": "igb",
"pinned_id": "0"
},
"eno2": {
@@ -189,6 +190,7 @@
"mac": "b4:2e:99:ac:ad:b5",
"name": "eno2",
"state": "UP",
+ "driver": "igb",
"pinned_id": "1"
},
"enp129s0f0np0": {
@@ -196,6 +198,7 @@
"mac": "1c:34:da:5c:5e:24",
"name": "enp129s0f0np0",
"state": "DOWN",
+ "driver": "mlx5_core",
"pinned_id": "2"
},
"enp129s0f1np1": {
@@ -203,6 +206,7 @@
"mac": "1c:34:da:5c:5e:25",
"name": "enp129s0f1np1",
"state": "DOWN",
+ "driver": "mlx5_core",
"pinned_id": "3"
},
"enp193s0f0np0": {
@@ -210,6 +214,7 @@
"mac": "24:8a:07:1e:05:bc",
"name": "enp193s0f0np0",
"state": "UP",
+ "driver": "mlx5_core",
"pinned_id": "4"
},
"enp193s0f1np1": {
@@ -217,6 +222,7 @@
"mac": "24:8a:07:1e:05:bd",
"name": "enp193s0f1np1",
"state": "DOWN",
+ "driver": "mlx5_core",
"pinned_id": "5"
},
"enp65s0f0": {
@@ -224,6 +230,7 @@
"mac": "a0:36:9f:0a:b3:82",
"name": "enp65s0f0",
"state": "DOWN",
+ "driver": "igb",
"pinned_id": "6"
},
"enp65s0f1": {
@@ -231,6 +238,7 @@
"mac": "a0:36:9f:0a:b3:83",
"name": "enp65s0f1",
"state": "DOWN",
+ "driver": "igb",
"pinned_id": "7"
},
"enx5a4732ddc747": {
@@ -238,6 +246,7 @@
"mac": "5a:47:32:dd:c7:47",
"name": "enx5a4732ddc747",
"state": "UNKNOWN",
+ "driver": "cdc_ether",
"pinned_id": "8"
}
},
diff --git a/proxmox-installer-common/src/options.rs b/proxmox-installer-common/src/options.rs
index 48c77c9..50fd74e 100644
--- a/proxmox-installer-common/src/options.rs
+++ b/proxmox-installer-common/src/options.rs
@@ -677,6 +677,7 @@ mod tests {
name: "eth0".to_owned(),
index: 0,
state: InterfaceState::Up,
+ driver: "dummy".to_owned(),
mac: "01:23:45:67:89:ab".to_owned(),
addresses: Some(vec![
CidrAddress::new(Ipv4Addr::new(192, 168, 0, 2), 24).unwrap(),
@@ -804,6 +805,7 @@ mod tests {
name: "eth0".to_owned(),
index: 0,
state: InterfaceState::Up,
+ driver: "dummy".to_owned(),
mac: "01:23:45:67:89:ab".to_owned(),
addresses: None,
},
diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs
index 66cea72..4fda39d 100644
--- a/proxmox-installer-common/src/setup.rs
+++ b/proxmox-installer-common/src/setup.rs
@@ -473,6 +473,9 @@ pub struct Interface {
pub state: InterfaceState,
+ /// Kernel driver name that claims this interface.
+ pub driver: String,
+
#[serde(default)]
#[serde(deserialize_with = "deserialize_cidr_list")]
pub addresses: Option<Vec<CidrAddress>>,
--
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] 21+ messages in thread
* [pve-devel] [PATCH installer v3 04/15] common: utils: fix clippy warnings
2025-11-11 13:59 [pve-devel] [PATCH installer v3 00/15] support network interface name pinning Christoph Heiss
` (2 preceding siblings ...)
2025-11-11 13:59 ` [pve-devel] [PATCH installer v3 03/15] run env: network: add kernel driver name to network interface info Christoph Heiss
@ 2025-11-11 13:59 ` Christoph Heiss
2025-11-11 13:59 ` [pve-devel] [PATCH installer v3 05/15] common: setup: simplify network address list serialization Christoph Heiss
` (13 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Christoph Heiss @ 2025-11-11 13:59 UTC (permalink / raw)
To: pve-devel
No functional changes.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v2 -> v3:
* applied new clippy lints (mostly folding if-let-chains)
Changes v1 -> v2:
* no changes
proxmox-auto-install-assistant/src/main.rs | 36 +++++++-------
.../src/bin/proxmox-auto-installer.rs | 5 +-
proxmox-auto-installer/src/utils.rs | 10 ++--
.../src/fetch_plugins/http.rs | 5 +-
.../src/fetch_plugins/partition.rs | 5 +-
proxmox-installer-common/src/options.rs | 31 +++++-------
proxmox-installer-common/src/utils.rs | 6 +--
proxmox-post-hook/src/main.rs | 11 +++--
proxmox-tui-installer/src/main.rs | 5 +-
proxmox-tui-installer/src/views/bootdisk.rs | 2 +-
proxmox-tui-installer/src/views/mod.rs | 49 ++++++++++---------
11 files changed, 77 insertions(+), 88 deletions(-)
diff --git a/proxmox-auto-install-assistant/src/main.rs b/proxmox-auto-install-assistant/src/main.rs
index c0d932c..e0f2435 100644
--- a/proxmox-auto-install-assistant/src/main.rs
+++ b/proxmox-auto-install-assistant/src/main.rs
@@ -576,11 +576,11 @@ fn validate_answer(args: &CommandValidateAnswerArgs) -> Result<()> {
if args.debug {
println!("Parsed data from answer file:\n{:#?}", answer);
}
- if args.verify_password {
- if let Err(err) = verify_hashed_password_interactive(&answer) {
- eprintln!("{err:#}");
- valid = false;
- }
+ if args.verify_password
+ && let Err(err) = verify_hashed_password_interactive(&answer)
+ {
+ eprintln!("{err:#}");
+ valid = false;
}
}
Err(err) => {
@@ -775,7 +775,7 @@ fn get_iso_uuid(iso: impl AsRef<Path>) -> Result<String> {
if line.starts_with("-volume_date uuid") {
uuid = line
.split(' ')
- .last()
+ .next_back()
.ok_or_else(|| format_err!("xorriso did behave unexpectedly"))?
.replace('\'', "")
.trim()
@@ -823,10 +823,10 @@ fn get_disks() -> Result<BTreeMap<String, BTreeMap<String, String>>> {
let mut name = filename;
let mut udev_props: BTreeMap<String, String> = BTreeMap::new();
for line in output.lines() {
- if let Some(prop) = line.strip_prefix(PROP_DEVTYP_PREFIX) {
- if prop != "disk" {
- continue 'outer;
- }
+ if let Some(prop) = line.strip_prefix(PROP_DEVTYP_PREFIX)
+ && prop != "disk"
+ {
+ continue 'outer;
}
if line.starts_with(PROP_CDROM) || line.starts_with(PROP_ISO9660_FS) {
@@ -837,10 +837,10 @@ fn get_disks() -> Result<BTreeMap<String, BTreeMap<String, String>>> {
name = prop.to_owned();
};
- if let Some(prop) = line.strip_prefix("E: ") {
- if let Some((key, val)) = prop.split_once('=') {
- udev_props.insert(key.to_owned(), val.to_owned());
- }
+ if let Some(prop) = line.strip_prefix("E: ")
+ && let Some((key, val)) = prop.split_once('=')
+ {
+ udev_props.insert(key.to_owned(), val.to_owned());
}
}
@@ -867,10 +867,10 @@ fn get_nics() -> Result<BTreeMap<String, BTreeMap<String, String>>> {
let mut udev_props: BTreeMap<String, String> = BTreeMap::new();
for line in output.lines() {
- if let Some(prop) = line.strip_prefix("E: ") {
- if let Some((key, val)) = prop.split_once('=') {
- udev_props.insert(key.to_owned(), val.to_owned());
- }
+ if let Some(prop) = line.strip_prefix("E: ")
+ && let Some((key, val)) = prop.split_once('=')
+ {
+ udev_props.insert(key.to_owned(), val.to_owned());
}
}
diff --git a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
index a42029f..467ef1b 100644
--- a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
+++ b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
@@ -122,11 +122,10 @@ fn main() -> ExitCode {
}
};
- if answer.global.reboot_on_error {
- if let Err(err) = File::create("/run/proxmox-reboot-on-error") {
+ 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}");
}
- }
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 7d42f2c..2d68829 100644
--- a/proxmox-auto-installer/src/utils.rs
+++ b/proxmox-auto-installer/src/utils.rs
@@ -409,11 +409,10 @@ pub fn verify_disks_settings(answer: &Answer) -> Result<()> {
}
}
- if let answer::FsOptions::LVM(lvm) = &answer.disks.fs_options {
- if let Some((swapsize, hdsize)) = lvm.swapsize.zip(lvm.hdsize) {
+ if let answer::FsOptions::LVM(lvm) = &answer.disks.fs_options
+ && let Some((swapsize, hdsize)) = lvm.swapsize.zip(lvm.hdsize) {
check_swapsize(swapsize, hdsize)?;
}
- }
Ok(())
}
@@ -421,11 +420,10 @@ pub fn verify_disks_settings(answer: &Answer) -> Result<()> {
pub fn verify_first_boot_settings(answer: &Answer) -> Result<()> {
info!("Verifying first boot settings");
- if let Some(first_boot) = &answer.first_boot {
- if first_boot.source == FirstBootHookSourceMode::FromUrl && first_boot.url.is_none() {
+ 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!");
}
- }
Ok(())
}
diff --git a/proxmox-fetch-answer/src/fetch_plugins/http.rs b/proxmox-fetch-answer/src/fetch_plugins/http.rs
index 9b3d15c..d6331d8 100644
--- a/proxmox-fetch-answer/src/fetch_plugins/http.rs
+++ b/proxmox-fetch-answer/src/fetch_plugins/http.rs
@@ -139,11 +139,10 @@ impl FetchFromHTTP {
fn get_search_domain() -> Result<String> {
info!("Retrieving default search domain.");
for line in read_to_string("/etc/resolv.conf")?.lines() {
- if let Some((key, value)) = line.split_once(' ') {
- if key == "search" {
+ if let Some((key, value)) = line.split_once(' ')
+ && 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 7a7b17a..f952513 100644
--- a/proxmox-fetch-answer/src/fetch_plugins/partition.rs
+++ b/proxmox-fetch-answer/src/fetch_plugins/partition.rs
@@ -122,11 +122,10 @@ fn mount_proxmoxinst_part(part_label: &str) -> Result<String> {
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) {
- if mp == target_path {
+ if let Some(mp) = line.split(' ').nth(1)
+ && mp == target_path {
return Ok(true);
}
- }
}
Ok(false)
}
diff --git a/proxmox-installer-common/src/options.rs b/proxmox-installer-common/src/options.rs
index 50fd74e..263bcfb 100644
--- a/proxmox-installer-common/src/options.rs
+++ b/proxmox-installer-common/src/options.rs
@@ -104,7 +104,7 @@ impl ZfsRaidLevel {
match self {
ZfsRaidLevel::Raid0 => {}
ZfsRaidLevel::Raid10 => {
- if disks.len() % 2 != 0 {
+ if !disks.len().is_multiple_of(2) {
return Err(format!(
"Needs an even number of disks, currently selected: {}",
disks.len(),
@@ -515,42 +515,35 @@ impl NetworkOptions {
if let Some(routes) = &network.routes {
let mut filled = false;
- if let Some(gw) = &routes.gateway4 {
- if let Some(iface) = network.interfaces.get(&gw.dev) {
+ if let Some(gw) = &routes.gateway4
+ && let Some(iface) = network.interfaces.get(&gw.dev) {
this.ifname.clone_from(&iface.name);
- if let Some(addresses) = &iface.addresses {
- if let Some(addr) = addresses.iter().find(|addr| addr.is_ipv4()) {
+ if let Some(addresses) = &iface.addresses
+ && let Some(addr) = addresses.iter().find(|addr| addr.is_ipv4()) {
this.gateway = gw.gateway;
this.address = addr.clone();
filled = true;
}
- }
}
- }
- if !filled {
- if let Some(gw) = &routes.gateway6 {
- if let Some(iface) = network.interfaces.get(&gw.dev) {
- if let Some(addresses) = &iface.addresses {
- if let Some(addr) = addresses.iter().find(|addr| addr.is_ipv6()) {
+ if !filled
+ && let Some(gw) = &routes.gateway6
+ && let Some(iface) = network.interfaces.get(&gw.dev)
+ && let Some(addresses) = &iface.addresses
+ && let Some(addr) = addresses.iter().find(|addr| addr.is_ipv6()) {
this.ifname.clone_from(&iface.name);
this.gateway = gw.gateway;
this.address = addr.clone();
}
- }
- }
- }
- }
}
// In case no there are no routes defined at all (e.g. no DHCP lease),
// try to set the interface name to *some* valid values. At least one
// NIC should always be present here, as the installation will abort
// earlier in that case, so use the first one enumerated.
- if this.ifname.is_empty() {
- if let Some(iface) = network.interfaces.values().min_by_key(|v| v.index) {
+ if this.ifname.is_empty()
+ && let Some(iface) = network.interfaces.values().min_by_key(|v| v.index) {
this.ifname.clone_from(&iface.name);
}
- }
this
}
diff --git a/proxmox-installer-common/src/utils.rs b/proxmox-installer-common/src/utils.rs
index 054a0fd..ffc862e 100644
--- a/proxmox-installer-common/src/utils.rs
+++ b/proxmox-installer-common/src/utils.rs
@@ -130,14 +130,14 @@ fn mask_limit(addr: &IpAddr) -> usize {
}
fn check_mask_limit(addr: &IpAddr, mask: usize) -> Result<(), CidrAddressParseError> {
- let limit = mask_limit(&addr);
- return if mask > limit {
+ let limit = mask_limit(addr);
+ if mask > limit {
Err(CidrAddressParseError::InvalidMask(
format!("mask cannot be greater than {limit}").into(),
))
} else {
Ok(())
- };
+ }
}
/// Possible errors that might occur when parsing FQDNs.
diff --git a/proxmox-post-hook/src/main.rs b/proxmox-post-hook/src/main.rs
index bd27121..3897c26 100644
--- a/proxmox-post-hook/src/main.rs
+++ b/proxmox-post-hook/src/main.rs
@@ -547,10 +547,11 @@ impl PostHookInfo {
for pkg in kernel_pkgs.lines() {
let parts = pkg.split('|').collect::<Vec<&str>>();
- if let [status, arch, name] = parts[..] {
- if status.trim() == "ii" && arch.trim() == dpkg_arch {
- return Ok(name.trim().to_owned());
- }
+ if let [status, arch, name] = parts[..]
+ && status.trim() == "ii"
+ && arch.trim() == dpkg_arch
+ {
+ return Ok(name.trim().to_owned());
}
}
@@ -612,7 +613,7 @@ impl PostHookInfo {
/// # Arguments
///
/// * `callback` - Callback to call with the absolute path where the chroot environment root is
-/// mounted.
+/// mounted.
fn with_chroot<R, F: FnOnce(&str) -> Result<R>>(callback: F) -> Result<R> {
let ec = Command::new("proxmox-chroot")
.arg("prepare")
diff --git a/proxmox-tui-installer/src/main.rs b/proxmox-tui-installer/src/main.rs
index 15ee5d3..3379e7c 100644
--- a/proxmox-tui-installer/src/main.rs
+++ b/proxmox-tui-installer/src/main.rs
@@ -188,11 +188,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) {
- if let Err(err) = system::set_keyboard_layout(kmap) {
+ 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}"));
}
- }
}
if state.runtime_info.total_memory < 1024 {
diff --git a/proxmox-tui-installer/src/views/bootdisk.rs b/proxmox-tui-installer/src/views/bootdisk.rs
index 9f6d235..7e3680b 100644
--- a/proxmox-tui-installer/src/views/bootdisk.rs
+++ b/proxmox-tui-installer/src/views/bootdisk.rs
@@ -183,7 +183,7 @@ impl AdvancedBootdiskOptionsView {
/// # Arguments
/// * `siv` - Cursive instance
/// * `fstype` - The chosen filesystem type by the user, for which the UI should be
- /// updated accordingly
+ /// updated accordingly
/// * `options_ref` - [`BootdiskOptionsRef`] where advanced disk options should be saved to
fn fstype_on_submit(siv: &mut Cursive, fstype: &FsType, options_ref: BootdiskOptionsRef) {
let state = siv.user_data::<InstallerState>().unwrap();
diff --git a/proxmox-tui-installer/src/views/mod.rs b/proxmox-tui-installer/src/views/mod.rs
index 537e3ed..3417191 100644
--- a/proxmox-tui-installer/src/views/mod.rs
+++ b/proxmox-tui-installer/src/views/mod.rs
@@ -109,10 +109,10 @@ impl<T: Copy + ToString + FromStr + PartialOrd> NumericEditView<T> {
pub fn get_content(&self) -> Result<T, <T as FromStr>::Err> {
let content = self.inner().get_content();
- if content.is_empty() {
- if let Some(placeholder) = self.placeholder {
- return Ok(placeholder);
- }
+ if content.is_empty()
+ && let Some(placeholder) = self.placeholder
+ {
+ return Ok(placeholder);
}
assert!(!(self.allow_empty && self.placeholder.is_none()));
@@ -137,17 +137,17 @@ impl<T: Copy + ToString + FromStr + PartialOrd> NumericEditView<T> {
fn check_bounds(&mut self, original: Arc<String>, result: EventResult) -> EventResult {
// Check if the new value is actually valid according to the max value, if set
- if let Some(max) = self.max_value {
- if let Ok(val) = self.get_content() {
- if result.is_consumed() && val > max {
- // Restore the original value, before the insert
- let cb = self.inner_mut().set_content((*original).clone());
- return EventResult::with_cb_once(move |siv| {
- result.process(siv);
- cb(siv);
- });
- }
- }
+ if let Some(max) = self.max_value
+ && let Ok(val) = self.get_content()
+ && result.is_consumed()
+ && val > max
+ {
+ // Restore the original value, before the insert
+ let cb = self.inner_mut().set_content((*original).clone());
+ return EventResult::with_cb_once(move |siv| {
+ result.process(siv);
+ cb(siv);
+ });
}
result
@@ -182,7 +182,7 @@ impl<T: Copy + ToString + FromStr + PartialOrd> NumericEditView<T> {
///
/// # Arguments
/// * `content` - New, stringified content for the inner [`EditView`]. Must be a valid value
- /// according to the container type `T`.
+ /// according to the container type `T`.
fn content_inner(mut self, content: &str) -> Self {
let mut inner = EditView::new();
std::mem::swap(self.inner_mut(), &mut inner);
@@ -198,15 +198,16 @@ impl<T: Copy + ToString + FromStr + PartialOrd> NumericEditView<T> {
fn wrap_draw_inner(&self, printer: &Printer) {
self.view.draw(printer);
- if self.inner().get_content().is_empty() && !printer.focused {
- if let Some(placeholder) = self.placeholder {
- let placeholder = placeholder.to_string();
+ if self.inner().get_content().is_empty()
+ && !printer.focused
+ && let Some(placeholder) = self.placeholder
+ {
+ let placeholder = placeholder.to_string();
- printer.with_color(
- (BaseColor::Blue.light(), BaseColor::Blue.dark()).into(),
- |printer| printer.print((0, 0), &placeholder),
- );
- }
+ printer.with_color(
+ (BaseColor::Blue.light(), BaseColor::Blue.dark()).into(),
+ |printer| printer.print((0, 0), &placeholder),
+ );
}
}
}
--
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] 21+ messages in thread
* [pve-devel] [PATCH installer v3 05/15] common: setup: simplify network address list serialization
2025-11-11 13:59 [pve-devel] [PATCH installer v3 00/15] support network interface name pinning Christoph Heiss
` (3 preceding siblings ...)
2025-11-11 13:59 ` [pve-devel] [PATCH installer v3 04/15] common: utils: fix clippy warnings Christoph Heiss
@ 2025-11-11 13:59 ` Christoph Heiss
2025-11-11 13:59 ` [pve-devel] [PATCH installer v3 06/15] common: implement support for `network_interface_pin_map` config Christoph Heiss
` (12 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Christoph Heiss @ 2025-11-11 13:59 UTC (permalink / raw)
To: pve-devel
Option<Vec<..>> is redundant in combination with #[serde(default)],
as the latter already defaults to an empty Vec<..> if the key does not
exist.
No functional changes.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v2 -> v3:
* rebased
Changes v1 -> v2:
* no changes
proxmox-installer-common/src/options.rs | 47 ++++++++++++-------------
proxmox-installer-common/src/setup.rs | 6 ++--
2 files changed, 25 insertions(+), 28 deletions(-)
diff --git a/proxmox-installer-common/src/options.rs b/proxmox-installer-common/src/options.rs
index 263bcfb..fa91060 100644
--- a/proxmox-installer-common/src/options.rs
+++ b/proxmox-installer-common/src/options.rs
@@ -514,26 +514,24 @@ impl NetworkOptions {
}
if let Some(routes) = &network.routes {
- let mut filled = false;
if let Some(gw) = &routes.gateway4
- && let Some(iface) = network.interfaces.get(&gw.dev) {
- this.ifname.clone_from(&iface.name);
- if let Some(addresses) = &iface.addresses
- && let Some(addr) = addresses.iter().find(|addr| addr.is_ipv4()) {
- this.gateway = gw.gateway;
- this.address = addr.clone();
- filled = true;
- }
+ && let Some(iface) = network.interfaces.get(&gw.dev)
+ {
+ // we got some ipv4 connectivity, so use that
+ this.ifname.clone_from(&iface.name);
+ if let Some(addr) = iface.addresses.iter().find(|addr| addr.is_ipv4()) {
+ this.gateway = gw.gateway;
+ this.address = addr.clone();
}
- if !filled
- && let Some(gw) = &routes.gateway6
- && let Some(iface) = network.interfaces.get(&gw.dev)
- && let Some(addresses) = &iface.addresses
- && let Some(addr) = addresses.iter().find(|addr| addr.is_ipv6()) {
- this.ifname.clone_from(&iface.name);
- this.gateway = gw.gateway;
- this.address = addr.clone();
- }
+ } else if let Some(gw) = &routes.gateway6
+ && let Some(iface) = network.interfaces.get(&gw.dev)
+ && let Some(addr) = iface.addresses.iter().find(|addr| addr.is_ipv6())
+ {
+ // no ipv4, but ipv6 connectivity
+ this.ifname.clone_from(&iface.name);
+ this.gateway = gw.gateway;
+ this.address = addr.clone();
+ }
}
// In case no there are no routes defined at all (e.g. no DHCP lease),
@@ -541,9 +539,10 @@ impl NetworkOptions {
// NIC should always be present here, as the installation will abort
// earlier in that case, so use the first one enumerated.
if this.ifname.is_empty()
- && let Some(iface) = network.interfaces.values().min_by_key(|v| v.index) {
- this.ifname.clone_from(&iface.name);
- }
+ && let Some(iface) = network.interfaces.values().min_by_key(|v| v.index)
+ {
+ this.ifname.clone_from(&iface.name);
+ }
this
}
@@ -672,9 +671,7 @@ mod tests {
state: InterfaceState::Up,
driver: "dummy".to_owned(),
mac: "01:23:45:67:89:ab".to_owned(),
- addresses: Some(vec![
- CidrAddress::new(Ipv4Addr::new(192, 168, 0, 2), 24).unwrap(),
- ]),
+ addresses: vec![CidrAddress::new(Ipv4Addr::new(192, 168, 0, 2), 24).unwrap()],
},
);
@@ -800,7 +797,7 @@ mod tests {
state: InterfaceState::Up,
driver: "dummy".to_owned(),
mac: "01:23:45:67:89:ab".to_owned(),
- addresses: None,
+ addresses: vec![],
},
);
diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs
index 4fda39d..4873fff 100644
--- a/proxmox-installer-common/src/setup.rs
+++ b/proxmox-installer-common/src/setup.rs
@@ -321,7 +321,7 @@ where
.collect())
}
-fn deserialize_cidr_list<'de, D>(deserializer: D) -> Result<Option<Vec<CidrAddress>>, D::Error>
+fn deserialize_cidr_list<'de, D>(deserializer: D) -> Result<Vec<CidrAddress>, D::Error>
where
D: Deserializer<'de>,
{
@@ -347,7 +347,7 @@ where
);
}
- Ok(Some(result))
+ Ok(result)
}
fn serialize_as_display<S, T>(value: &T, serializer: S) -> Result<S::Ok, S::Error>
@@ -478,7 +478,7 @@ pub struct Interface {
#[serde(default)]
#[serde(deserialize_with = "deserialize_cidr_list")]
- pub addresses: Option<Vec<CidrAddress>>,
+ pub addresses: Vec<CidrAddress>,
}
impl Interface {
--
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] 21+ messages in thread
* [pve-devel] [PATCH installer v3 06/15] common: implement support for `network_interface_pin_map` config
2025-11-11 13:59 [pve-devel] [PATCH installer v3 00/15] support network interface name pinning Christoph Heiss
` (4 preceding siblings ...)
2025-11-11 13:59 ` [pve-devel] [PATCH installer v3 05/15] common: setup: simplify network address list serialization Christoph Heiss
@ 2025-11-11 13:59 ` Christoph Heiss
2025-11-11 13:59 ` [pve-devel] [PATCH installer v3 07/15] auto: add support for pinning network interface names Christoph Heiss
` (11 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Christoph Heiss @ 2025-11-11 13:59 UTC (permalink / raw)
To: pve-devel
Adds all the pieces for installer frontends to wire up pinning support,
i.e. deserializing from the runtime environment, doing verification and
serializing it out to the low-level installer.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v2 -> v3:
* rebased
* switched fully-numeric check before first character digit check
* simplified first character digit check
* added more unit tests (thanks @Michael)
Changes v1 -> v2:
* improved validation for interface names, now according to our
`pve-iface` json schema
proxmox-auto-installer/src/utils.rs | 3 +-
proxmox-installer-common/src/lib.rs | 5 +
proxmox-installer-common/src/options.rs | 242 ++++++++++++++++++++++--
proxmox-installer-common/src/setup.rs | 51 ++++-
proxmox-tui-installer/src/setup.rs | 3 +-
5 files changed, 279 insertions(+), 25 deletions(-)
diff --git a/proxmox-auto-installer/src/utils.rs b/proxmox-auto-installer/src/utils.rs
index 2d68829..ce49baa 100644
--- a/proxmox-auto-installer/src/utils.rs
+++ b/proxmox-auto-installer/src/utils.rs
@@ -2,7 +2,7 @@ use anyhow::{Context, Result, bail};
use glob::Pattern;
use log::info;
use std::{
- collections::{BTreeMap, HashSet},
+ collections::{BTreeMap, HashMap, HashSet},
process::Command,
};
@@ -483,6 +483,7 @@ pub fn parse_answer(
root_ssh_keys: answer.global.root_ssh_keys.clone(),
mngmt_nic: network_settings.ifname,
+ network_interface_pin_map: HashMap::new(),
hostname: network_settings
.fqdn
diff --git a/proxmox-installer-common/src/lib.rs b/proxmox-installer-common/src/lib.rs
index ea907a0..a85d5f8 100644
--- a/proxmox-installer-common/src/lib.rs
+++ b/proxmox-installer-common/src/lib.rs
@@ -10,6 +10,11 @@ pub mod http;
#[cfg(feature = "cli")]
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
+}
+
pub const RUNTIME_DIR: &str = "/run/proxmox-installer";
/// Default placeholder value for the administrator email address.
diff --git a/proxmox-installer-common/src/options.rs b/proxmox-installer-common/src/options.rs
index fa91060..0d72f54 100644
--- a/proxmox-installer-common/src/options.rs
+++ b/proxmox-installer-common/src/options.rs
@@ -1,12 +1,14 @@
use anyhow::{Result, bail};
use regex::Regex;
use serde::{Deserialize, Serialize};
+use std::collections::HashMap;
use std::net::{IpAddr, Ipv4Addr};
use std::str::FromStr;
use std::sync::OnceLock;
use std::{cmp, fmt};
use crate::disk_checks::check_raid_min_disks;
+use crate::net::MAX_IFNAME_LEN;
use crate::setup::{LocaleInfo, NetworkInfo, RuntimeInfo, SetupInfo};
use crate::utils::{CidrAddress, Fqdn};
@@ -476,6 +478,75 @@ impl TimezoneOptions {
}
}
+/// Options controlling the behaviour of the network interface pinning (by
+/// creating appropriate systemd.link files) during the installation.
+#[derive(Clone, Debug, Default, PartialEq, Deserialize)]
+#[serde(rename_all = "kebab-case", deny_unknown_fields)]
+pub struct NetworkInterfacePinningOptions {
+ /// Maps MAC address to custom name
+ #[serde(default)]
+ pub mapping: HashMap<String, String>,
+}
+
+impl NetworkInterfacePinningOptions {
+ /// Default prefix to prepend to the pinned interface ID as received from the low-level
+ /// installer.
+ pub const DEFAULT_PREFIX: &str = "nic";
+
+ /// Does some basic checks on the options.
+ ///
+ /// This includes checks for:
+ /// - empty interface names
+ /// - overlong interface names
+ /// - duplicate interface names
+ /// - only contains ASCII alphanumeric characters and underscore, as
+ /// enforced by our `pve-iface` json schema.
+ 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() > MAX_IFNAME_LEN {
+ bail!(
+ "interface name '{name}' for '{mac}' cannot be longer than {} characters",
+ MAX_IFNAME_LEN
+ );
+ }
+
+ if name.chars().all(char::is_numeric) {
+ bail!(
+ "interface name '{name}' for '{mac}' is invalid: name must not be fully numeric"
+ );
+ }
+
+ // Mimicking the `pve-iface` schema verification
+ if name.starts_with(|c: char| c.is_ascii_digit()) {
+ bail!(
+ "interface name '{name}' for '{mac}' is invalid: name must not start with a number"
+ );
+ }
+
+ if !name.chars().all(|c| c.is_ascii_alphanumeric() || c == '_') {
+ bail!(
+ "interface name '{name}' for '{mac}' is invalid: name must only consist of alphanumeric characters and underscores"
+ );
+ }
+
+ if let Some(duplicate_mac) = reverse_mapping.get(name)
+ && mac != duplicate_mac
+ {
+ bail!("duplicate interface name mapping '{name}' for: {mac}, {duplicate_mac}");
+ }
+
+ reverse_mapping.insert(name.clone(), mac.clone());
+ }
+
+ Ok(())
+ }
+}
+
#[derive(Clone, Debug, PartialEq)]
pub struct NetworkOptions {
pub ifname: String,
@@ -483,6 +554,7 @@ pub struct NetworkOptions {
pub address: CidrAddress,
pub gateway: IpAddr,
pub dns_server: IpAddr,
+ pub pinning_opts: Option<NetworkInterfacePinningOptions>,
}
impl NetworkOptions {
@@ -492,6 +564,7 @@ impl NetworkOptions {
setup: &SetupInfo,
network: &NetworkInfo,
default_domain: Option<&str>,
+ pinning_opts: Option<&NetworkInterfacePinningOptions>,
) -> Self {
// Sets up sensible defaults as much as possible, such that even in the
// worse case nothing breaks down *completely*.
@@ -507,28 +580,39 @@ impl NetworkOptions {
address: CidrAddress::new(Ipv4Addr::new(192, 168, 100, 2), 24).unwrap(),
gateway: Ipv4Addr::new(192, 168, 100, 1).into(),
dns_server: Ipv4Addr::new(192, 168, 100, 1).into(),
+ pinning_opts: pinning_opts.cloned(),
};
if let Some(ip) = network.dns.dns.first() {
this.dns_server = *ip;
}
- if let Some(routes) = &network.routes {
- if let Some(gw) = &routes.gateway4
- && let Some(iface) = network.interfaces.get(&gw.dev)
- {
- // we got some ipv4 connectivity, so use that
+ if let Some(routes) = &network.routes
+ && let Some(gw) = &routes.gateway4
+ && let Some(iface) = network.interfaces.get(&gw.dev)
+ {
+ // we got some ipv4 connectivity, so use that
+
+ if let Some(opts) = pinning_opts {
+ this.ifname.clone_from(&iface.to_pinned(opts).name);
+ } else {
this.ifname.clone_from(&iface.name);
- if let Some(addr) = iface.addresses.iter().find(|addr| addr.is_ipv4()) {
- this.gateway = gw.gateway;
- this.address = addr.clone();
- }
+ }
+
+ if let Some(addr) = iface.addresses.iter().find(|addr| addr.is_ipv4()) {
+ this.gateway = gw.gateway;
+ this.address = addr.clone();
} else if let Some(gw) = &routes.gateway6
&& let Some(iface) = network.interfaces.get(&gw.dev)
&& let Some(addr) = iface.addresses.iter().find(|addr| addr.is_ipv6())
{
// no ipv4, but ipv6 connectivity
- this.ifname.clone_from(&iface.name);
+ if let Some(opts) = pinning_opts {
+ this.ifname.clone_from(&iface.to_pinned(opts).name);
+ } else {
+ this.ifname.clone_from(&iface.name);
+ }
+
this.gateway = gw.gateway;
this.address = addr.clone();
}
@@ -541,7 +625,20 @@ impl NetworkOptions {
if this.ifname.is_empty()
&& let Some(iface) = network.interfaces.values().min_by_key(|v| v.index)
{
- this.ifname.clone_from(&iface.name);
+ if let Some(opts) = pinning_opts {
+ this.ifname.clone_from(&iface.to_pinned(opts).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
+ for iface in network.interfaces.values() {
+ let pinned_name = iface.to_pinned(opts).name;
+ opts.mapping.entry(iface.mac.clone()).or_insert(pinned_name);
+ }
}
this
@@ -668,6 +765,7 @@ mod tests {
Interface {
name: "eth0".to_owned(),
index: 0,
+ pinned_id: "0".to_owned(),
state: InterfaceState::Up,
driver: "dummy".to_owned(),
mac: "01:23:45:67:89:ab".to_owned(),
@@ -699,49 +797,53 @@ mod tests {
let (setup, mut info) = mock_setup_network();
pretty_assertions::assert_eq!(
- NetworkOptions::defaults_from(&setup, &info, None),
+ NetworkOptions::defaults_from(&setup, &info, None, None),
NetworkOptions {
ifname: "eth0".to_owned(),
fqdn: Fqdn::from("foo.bar.com").unwrap(),
address: CidrAddress::new(Ipv4Addr::new(192, 168, 0, 2), 24).unwrap(),
gateway: IpAddr::V4(Ipv4Addr::new(192, 168, 0, 1)),
dns_server: Ipv4Addr::new(192, 168, 100, 1).into(),
+ pinning_opts: None,
}
);
info.hostname = None;
pretty_assertions::assert_eq!(
- NetworkOptions::defaults_from(&setup, &info, None),
+ NetworkOptions::defaults_from(&setup, &info, None, None),
NetworkOptions {
ifname: "eth0".to_owned(),
fqdn: Fqdn::from("pve.bar.com").unwrap(),
address: CidrAddress::new(Ipv4Addr::new(192, 168, 0, 2), 24).unwrap(),
gateway: IpAddr::V4(Ipv4Addr::new(192, 168, 0, 1)),
dns_server: Ipv4Addr::new(192, 168, 100, 1).into(),
+ pinning_opts: None,
}
);
info.dns.domain = None;
pretty_assertions::assert_eq!(
- NetworkOptions::defaults_from(&setup, &info, None),
+ NetworkOptions::defaults_from(&setup, &info, None, None),
NetworkOptions {
ifname: "eth0".to_owned(),
fqdn: Fqdn::from("pve.example.invalid").unwrap(),
address: CidrAddress::new(Ipv4Addr::new(192, 168, 0, 2), 24).unwrap(),
gateway: IpAddr::V4(Ipv4Addr::new(192, 168, 0, 1)),
dns_server: Ipv4Addr::new(192, 168, 100, 1).into(),
+ pinning_opts: None,
}
);
info.hostname = Some("foo".to_owned());
pretty_assertions::assert_eq!(
- NetworkOptions::defaults_from(&setup, &info, None),
+ NetworkOptions::defaults_from(&setup, &info, None, None),
NetworkOptions {
ifname: "eth0".to_owned(),
fqdn: Fqdn::from("foo.example.invalid").unwrap(),
address: CidrAddress::new(Ipv4Addr::new(192, 168, 0, 2), 24).unwrap(),
gateway: IpAddr::V4(Ipv4Addr::new(192, 168, 0, 1)),
dns_server: Ipv4Addr::new(192, 168, 100, 1).into(),
+ pinning_opts: None,
}
);
}
@@ -751,37 +853,40 @@ mod tests {
let (setup, mut info) = mock_setup_network();
pretty_assertions::assert_eq!(
- NetworkOptions::defaults_from(&setup, &info, None),
+ NetworkOptions::defaults_from(&setup, &info, None, None),
NetworkOptions {
ifname: "eth0".to_owned(),
fqdn: Fqdn::from("foo.bar.com").unwrap(),
address: CidrAddress::new(Ipv4Addr::new(192, 168, 0, 2), 24).unwrap(),
gateway: IpAddr::V4(Ipv4Addr::new(192, 168, 0, 1)),
dns_server: Ipv4Addr::new(192, 168, 100, 1).into(),
+ pinning_opts: None,
}
);
info.dns.domain = None;
pretty_assertions::assert_eq!(
- NetworkOptions::defaults_from(&setup, &info, Some("custom.local")),
+ NetworkOptions::defaults_from(&setup, &info, Some("custom.local"), None),
NetworkOptions {
ifname: "eth0".to_owned(),
fqdn: Fqdn::from("foo.custom.local").unwrap(),
address: CidrAddress::new(Ipv4Addr::new(192, 168, 0, 2), 24).unwrap(),
gateway: IpAddr::V4(Ipv4Addr::new(192, 168, 0, 1)),
dns_server: Ipv4Addr::new(192, 168, 100, 1).into(),
+ pinning_opts: None,
}
);
info.dns.domain = Some("some.domain.local".to_owned());
pretty_assertions::assert_eq!(
- NetworkOptions::defaults_from(&setup, &info, Some("custom.local")),
+ NetworkOptions::defaults_from(&setup, &info, Some("custom.local"), None),
NetworkOptions {
ifname: "eth0".to_owned(),
fqdn: Fqdn::from("foo.custom.local").unwrap(),
address: CidrAddress::new(Ipv4Addr::new(192, 168, 0, 2), 24).unwrap(),
gateway: IpAddr::V4(Ipv4Addr::new(192, 168, 0, 1)),
dns_server: Ipv4Addr::new(192, 168, 100, 1).into(),
+ pinning_opts: None,
}
);
}
@@ -794,6 +899,7 @@ mod tests {
Interface {
name: "eth0".to_owned(),
index: 0,
+ pinned_id: "0".to_owned(),
state: InterfaceState::Up,
driver: "dummy".to_owned(),
mac: "01:23:45:67:89:ab".to_owned(),
@@ -814,14 +920,112 @@ mod tests {
let setup = SetupInfo::mocked();
pretty_assertions::assert_eq!(
- NetworkOptions::defaults_from(&setup, &info, None),
+ NetworkOptions::defaults_from(&setup, &info, None, None),
NetworkOptions {
ifname: "eth0".to_owned(),
fqdn: Fqdn::from("pve.example.invalid").unwrap(),
address: CidrAddress::new(Ipv4Addr::new(192, 168, 100, 2), 24).unwrap(),
gateway: IpAddr::V4(Ipv4Addr::new(192, 168, 100, 1)),
dns_server: Ipv4Addr::new(192, 168, 100, 1).into(),
+ pinning_opts: None,
}
);
}
+
+ #[test]
+ fn network_interface_pinning_options_fail_on_empty_name() {
+ let mut options = NetworkInterfacePinningOptions::default();
+ options
+ .mapping
+ .insert("ab:cd:ef:12:34:56".to_owned(), String::new());
+
+ let res = options.verify();
+ assert!(res.is_err());
+ assert_eq!(
+ res.unwrap_err().to_string(),
+ "interface name for 'ab:cd:ef:12:34:56' cannot be empty"
+ )
+ }
+
+ #[test]
+ fn network_interface_pinning_options_fail_on_overlong_name() {
+ let mut options = NetworkInterfacePinningOptions::default();
+ options.mapping.insert(
+ "ab:cd:ef:12:34:56".to_owned(),
+ "waytoolonginterfacename".to_owned(),
+ );
+
+ let res = options.verify();
+ assert!(res.is_err());
+ assert_eq!(
+ res.unwrap_err().to_string(),
+ "interface name 'waytoolonginterfacename' for 'ab:cd:ef:12:34:56' cannot be longer than 15 characters"
+ )
+ }
+
+ #[test]
+ fn network_interface_pinning_options_fail_on_duplicate_name() {
+ let mut options = NetworkInterfacePinningOptions::default();
+ options
+ .mapping
+ .insert("ab:cd:ef:12:34:56".to_owned(), "nic0".to_owned());
+ options
+ .mapping
+ .insert("12:34:56:ab:cd:ef".to_owned(), "nic0".to_owned());
+
+ let res = options.verify();
+ assert!(res.is_err());
+ let err = res.unwrap_err().to_string();
+
+ // [HashMap] does not guarantee iteration order, so just check for the substrings
+ // we expect to find
+ assert!(err.contains("duplicate interface name mapping 'nic0' for: "));
+ assert!(err.contains("12:34:56:ab:cd:ef"));
+ assert!(err.contains("ab:cd:ef:12:34:56"));
+ }
+
+ #[test]
+ fn network_interface_pinning_options_fail_on_invalid_characters() {
+ let mut options = NetworkInterfacePinningOptions::default();
+ options
+ .mapping
+ .insert("ab:cd:ef:12:34:56".to_owned(), "nic-".to_owned());
+
+ let res = options.verify();
+ assert!(res.is_err());
+ assert_eq!(
+ res.unwrap_err().to_string(),
+ "interface name 'nic-' for 'ab:cd:ef:12:34:56' is invalid: name must only consist of alphanumeric characters and underscores"
+ )
+ }
+
+ #[test]
+ fn network_interface_pinning_options_fail_on_name_starting_with_number() {
+ let mut options = NetworkInterfacePinningOptions::default();
+ options
+ .mapping
+ .insert("ab:cd:ef:12:34:56".to_owned(), "0nic".to_owned());
+
+ let res = options.verify();
+ 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"
+ )
+ }
+
+ #[test]
+ fn network_interface_pinning_options_fail_on_fully_numeric_name() {
+ let mut options = NetworkInterfacePinningOptions::default();
+ options
+ .mapping
+ .insert("ab:cd:ef:12:34:56".to_owned(), "12345".to_owned());
+
+ let res = options.verify();
+ assert!(res.is_err());
+ assert_eq!(
+ res.unwrap_err().to_string(),
+ "interface name '12345' for 'ab:cd:ef:12:34:56' is invalid: name must not be fully numeric"
+ )
+ }
}
diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs
index 4873fff..3e99576 100644
--- a/proxmox-installer-common/src/setup.rs
+++ b/proxmox-installer-common/src/setup.rs
@@ -3,6 +3,7 @@ use std::{
collections::{BTreeMap, HashMap},
fmt,
fs::File,
+ hash::Hash,
io::{self, BufReader},
net::IpAddr,
path::{Path, PathBuf},
@@ -13,8 +14,8 @@ use serde::{Deserialize, Deserializer, Serialize, Serializer, de};
use crate::{
options::{
- BtrfsBootdiskOptions, BtrfsCompressOption, Disk, FsType, ZfsBootdiskOptions,
- ZfsChecksumOption, ZfsCompressOption,
+ BtrfsBootdiskOptions, BtrfsCompressOption, Disk, FsType, NetworkInterfacePinningOptions,
+ ZfsBootdiskOptions, ZfsChecksumOption, ZfsCompressOption,
},
utils::CidrAddress,
};
@@ -443,8 +444,9 @@ pub struct Gateway {
pub gateway: IpAddr,
}
-#[derive(Clone, Deserialize)]
+#[derive(Clone, Deserialize, Serialize)]
#[serde(rename_all = "UPPERCASE")]
+/// The current interface state as reported by the kernel.
pub enum InterfaceState {
Up,
Down,
@@ -452,6 +454,8 @@ pub enum InterfaceState {
Unknown,
}
+serde_plain::derive_display_from_serialize!(InterfaceState);
+
impl InterfaceState {
// avoid display trait as this is not the string representation for a serializer
pub fn render(&self) -> String {
@@ -469,6 +473,9 @@ pub struct Interface {
pub index: usize,
+ /// Sequential interface ID for pinning interface names.
+ pub pinned_id: String,
+
pub mac: String,
pub state: InterfaceState,
@@ -484,7 +491,22 @@ pub struct Interface {
impl Interface {
// avoid display trait as this is not the string representation for a serializer
pub fn render(&self) -> String {
- format!("{} {}", self.state.render(), self.name)
+ format!("{} {} ({})", self.state.render(), self.name, self.mac)
+ }
+
+ 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();
+
+ this
}
}
@@ -577,6 +599,14 @@ pub struct InstallConfig {
pub root_ssh_keys: Vec<String>,
pub mngmt_nic: String,
+ // Maps MAC addresses -> custom name. If set, enables pinning for all
+ // interfaces present.
+ #[serde(
+ default,
+ skip_serializing_if = "HashMap::is_empty",
+ deserialize_with = "deserialize_optional_map"
+ )]
+ pub network_interface_pin_map: HashMap<String, String>,
pub hostname: String,
pub domain: String,
@@ -610,3 +640,16 @@ pub enum LowLevelMessage {
text: Option<String>,
},
}
+
+/// Deserializes an optional [HashMap].
+///
+/// If missing, it returns an empty map, otherwise the deserialized map.
+fn deserialize_optional_map<'de, D, K, V>(deserializer: D) -> Result<HashMap<K, V>, D::Error>
+where
+ D: Deserializer<'de>,
+ K: Eq + Hash + Deserialize<'de>,
+ V: Deserialize<'de>,
+{
+ let map: Option<HashMap<K, V>> = Deserialize::deserialize(deserializer)?;
+ Ok(map.unwrap_or_default())
+}
diff --git a/proxmox-tui-installer/src/setup.rs b/proxmox-tui-installer/src/setup.rs
index b90c7dc..d2ffb70 100644
--- a/proxmox-tui-installer/src/setup.rs
+++ b/proxmox-tui-installer/src/setup.rs
@@ -1,4 +1,4 @@
-use std::collections::BTreeMap;
+use std::collections::{BTreeMap, HashMap};
use crate::options::InstallerOptions;
use proxmox_installer_common::{
@@ -32,6 +32,7 @@ impl From<InstallerOptions> for InstallConfig {
root_ssh_keys: vec![],
mngmt_nic: options.network.ifname,
+ network_interface_pin_map: HashMap::new(),
// Safety: At this point, it is know that we have a valid FQDN, as
// this is set by the TUI network panel, which only lets the user
--
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] 21+ messages in thread
* [pve-devel] [PATCH installer v3 07/15] auto: add support for pinning network interface names
2025-11-11 13:59 [pve-devel] [PATCH installer v3 00/15] support network interface name pinning Christoph Heiss
` (5 preceding siblings ...)
2025-11-11 13:59 ` [pve-devel] [PATCH installer v3 06/15] common: implement support for `network_interface_pin_map` config Christoph Heiss
@ 2025-11-11 13:59 ` Christoph Heiss
2025-11-11 13:59 ` [pve-devel] [PATCH installer v3 08/15] assistant: verify network settings in `validate-answer` subcommand Christoph Heiss
` (10 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Christoph Heiss @ 2025-11-11 13:59 UTC (permalink / raw)
To: pve-devel
Introduce a new `[network.interface-name-pinning]` section in the answer
file, which is just a (TOML) table mapping MAC addresses to interface
names.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v2 -> v3:
* no changes
Changes v1 -> v2:
* added mac address validation upon installation start, i.e. issuing
a warning if an unknown mac address is specified in the pinning
mapping table
proxmox-auto-installer/src/answer.rs | 63 ++++++++++++++-----
proxmox-auto-installer/src/utils.rs | 57 +++++++++++++++--
proxmox-auto-installer/tests/parse-answer.rs | 2 +
.../network_interface_pinning.json | 30 +++++++++
.../network_interface_pinning.toml | 22 +++++++
...rface_pinning_overlong_interface_name.json | 3 +
...rface_pinning_overlong_interface_name.toml | 18 ++++++
7 files changed, 175 insertions(+), 20 deletions(-)
create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/network_interface_pinning.json
create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/network_interface_pinning.toml
create mode 100644 proxmox-auto-installer/tests/resources/parse_answer_fail/network_interface_pinning_overlong_interface_name.json
create mode 100644 proxmox-auto-installer/tests/resources/parse_answer_fail/network_interface_pinning_overlong_interface_name.toml
diff --git a/proxmox-auto-installer/src/answer.rs b/proxmox-auto-installer/src/answer.rs
index 88f4c87..1e455ca 100644
--- a/proxmox-auto-installer/src/answer.rs
+++ b/proxmox-auto-installer/src/answer.rs
@@ -1,13 +1,17 @@
-use anyhow::{Result, format_err};
+use anyhow::{Result, bail, format_err};
use proxmox_installer_common::{
options::{
- BtrfsCompressOption, BtrfsRaidLevel, FsType, ZfsChecksumOption, ZfsCompressOption,
- ZfsRaidLevel,
+ BtrfsCompressOption, BtrfsRaidLevel, FsType, NetworkInterfacePinningOptions,
+ ZfsChecksumOption, ZfsCompressOption, ZfsRaidLevel,
},
utils::{CidrAddress, Fqdn},
};
use serde::{Deserialize, Serialize};
-use std::{collections::BTreeMap, io::BufRead, net::IpAddr};
+use std::{
+ collections::{BTreeMap, HashMap},
+ io::BufRead,
+ net::IpAddr,
+};
// NOTE New answer file properties must use kebab-case, but should allow snake_case for backwards
// compatibility. TODO Remove the snake_cased variants in a future major version (e.g. PVE 10).
@@ -178,6 +182,18 @@ enum NetworkConfigMode {
FromAnswer,
}
+/// Options controlling the behaviour of the network interface pinning (by
+/// creating appropriate systemd.link files) during the installation.
+#[derive(Clone, Debug, Default, PartialEq, Deserialize)]
+#[serde(rename_all = "kebab-case", deny_unknown_fields)]
+pub struct NetworkInterfacePinningOptionsAnswer {
+ /// Whether interfaces should be pinned during the installation.
+ pub enabled: bool,
+ /// Maps MAC address to custom name
+ #[serde(default)]
+ pub mapping: HashMap<String, String>,
+}
+
#[derive(Clone, Deserialize, Debug)]
#[serde(rename_all = "kebab-case", deny_unknown_fields)]
struct NetworkInAnswer {
@@ -188,30 +204,47 @@ struct NetworkInAnswer {
pub gateway: Option<IpAddr>,
#[serde(default)]
pub filter: BTreeMap<String, String>,
+ /// Controls network interface pinning behaviour during installation.
+ /// Off by default. Allowed for both `from-dhcp` and `from-answer` modes.
+ #[serde(default)]
+ pub interface_name_pinning: Option<NetworkInterfacePinningOptionsAnswer>,
}
#[derive(Clone, Deserialize, Debug)]
#[serde(try_from = "NetworkInAnswer", deny_unknown_fields)]
pub struct Network {
pub network_settings: NetworkSettings,
+ /// Controls network interface pinning behaviour during installation.
+ pub interface_name_pinning: Option<NetworkInterfacePinningOptions>,
}
impl TryFrom<NetworkInAnswer> for Network {
- type Error = &'static str;
+ type Error = anyhow::Error;
+
+ fn try_from(network: NetworkInAnswer) -> Result<Self> {
+ let interface_name_pinning = match network.interface_name_pinning {
+ Some(opts) if opts.enabled => {
+ let opts = NetworkInterfacePinningOptions {
+ mapping: opts.mapping,
+ };
+ opts.verify()?;
+ Some(opts)
+ }
+ _ => None,
+ };
- fn try_from(network: NetworkInAnswer) -> Result<Self, Self::Error> {
if network.source == NetworkConfigMode::FromAnswer {
if network.cidr.is_none() {
- return Err("Field 'cidr' must be set.");
+ bail!("Field 'cidr' must be set.");
}
if network.dns.is_none() {
- return Err("Field 'dns' must be set.");
+ bail!("Field 'dns' must be set.");
}
if network.gateway.is_none() {
- return Err("Field 'gateway' must be set.");
+ bail!("Field 'gateway' must be set.");
}
if network.filter.is_empty() {
- return Err("Field 'filter' must be set.");
+ bail!("Field 'filter' must be set.");
}
Ok(Network {
@@ -221,23 +254,25 @@ impl TryFrom<NetworkInAnswer> for Network {
gateway: network.gateway.unwrap(),
filter: network.filter,
}),
+ interface_name_pinning,
})
} else {
if network.cidr.is_some() {
- return Err("Field 'cidr' not supported for 'from-dhcp' config.");
+ bail!("Field 'cidr' not supported for 'from-dhcp' config.");
}
if network.dns.is_some() {
- return Err("Field 'dns' not supported for 'from-dhcp' config.");
+ bail!("Field 'dns' not supported for 'from-dhcp' config.");
}
if network.gateway.is_some() {
- return Err("Field 'gateway' not supported for 'from-dhcp' config.");
+ bail!("Field 'gateway' not supported for 'from-dhcp' config.");
}
if !network.filter.is_empty() {
- return Err("Field 'filter' not supported for 'from-dhcp' config.");
+ bail!("Field 'filter' not supported for 'from-dhcp' config.");
}
Ok(Network {
network_settings: NetworkSettings::FromDhcp,
+ interface_name_pinning,
})
}
}
diff --git a/proxmox-auto-installer/src/utils.rs b/proxmox-auto-installer/src/utils.rs
index ce49baa..f5c3f70 100644
--- a/proxmox-auto-installer/src/utils.rs
+++ b/proxmox-auto-installer/src/utils.rs
@@ -1,15 +1,15 @@
use anyhow::{Context, Result, bail};
use glob::Pattern;
-use log::info;
+use log::{info, warn};
use std::{
- collections::{BTreeMap, HashMap, HashSet},
+ collections::{BTreeMap, HashSet},
process::Command,
};
use crate::{
answer::{
self, Answer, DiskSelection, FirstBootHookSourceMode, FqdnConfig, FqdnExtendedConfig,
- FqdnSourceMode,
+ FqdnSourceMode, Network,
},
udevinfo::UdevInfo,
};
@@ -35,7 +35,12 @@ fn get_network_settings(
let mut network_options = match &answer.global.fqdn {
// If the user set a static FQDN in the answer file, override it
FqdnConfig::Simple(name) => {
- let mut opts = NetworkOptions::defaults_from(setup_info, &runtime_info.network, None);
+ let mut opts = NetworkOptions::defaults_from(
+ setup_info,
+ &runtime_info.network,
+ None,
+ answer.network.interface_name_pinning.as_ref(),
+ );
opts.fqdn = name.to_owned();
opts
}
@@ -58,7 +63,12 @@ fn get_network_settings(
bail!("no domain received from DHCP server and `global.fqdn.domain` is unset!");
}
- NetworkOptions::defaults_from(setup_info, &runtime_info.network, domain.as_deref())
+ NetworkOptions::defaults_from(
+ setup_info,
+ &runtime_info.network,
+ domain.as_deref(),
+ answer.network.interface_name_pinning.as_ref(),
+ )
}
};
@@ -68,6 +78,12 @@ fn get_network_settings(
network_options.gateway = settings.gateway;
network_options.ifname = get_single_udev_index(&settings.filter, &udev_info.nics)?;
}
+
+ if let Some(opts) = &network_options.pinning_opts {
+ info!("Network interface name pinning is enabled");
+ opts.verify()?;
+ }
+
info!("Network interface used is '{}'", &network_options.ifname);
Ok(network_options)
}
@@ -428,6 +444,31 @@ pub fn verify_first_boot_settings(answer: &Answer) -> Result<()> {
Ok(())
}
+pub fn verify_network_settings(network: &Network, run_env: Option<&RuntimeInfo>) -> Result<()> {
+ info!("Verifying network settings");
+
+ if let Some(pin_opts) = &network.interface_name_pinning {
+ pin_opts.verify()?;
+
+ if let Some(run_env) = run_env {
+ for (mac, name) in pin_opts.mapping.iter() {
+ if !run_env
+ .network
+ .interfaces
+ .values()
+ .any(|iface| iface.mac == *mac)
+ {
+ warn!(
+ "found unknown address '{mac}' (mapped to '{name}') in network interface pinning options"
+ );
+ }
+ }
+ }
+ }
+
+ Ok(())
+}
+
pub fn parse_answer(
answer: &Answer,
udev_info: &UdevInfo,
@@ -449,6 +490,7 @@ pub fn parse_answer(
verify_disks_settings(answer)?;
verify_email_and_root_password_settings(answer)?;
verify_first_boot_settings(answer)?;
+ verify_network_settings(&answer.network, Some(runtime_info))?;
let root_password = match (
&answer.global.root_password,
@@ -483,7 +525,10 @@ pub fn parse_answer(
root_ssh_keys: answer.global.root_ssh_keys.clone(),
mngmt_nic: network_settings.ifname,
- network_interface_pin_map: HashMap::new(),
+ network_interface_pin_map: network_settings
+ .pinning_opts
+ .map(|o| o.mapping)
+ .unwrap_or_default(),
hostname: network_settings
.fqdn
diff --git a/proxmox-auto-installer/tests/parse-answer.rs b/proxmox-auto-installer/tests/parse-answer.rs
index 6754374..696fe1f 100644
--- a/proxmox-auto-installer/tests/parse-answer.rs
+++ b/proxmox-auto-installer/tests/parse-answer.rs
@@ -129,6 +129,7 @@ mod tests {
full_fqdn_from_dhcp_with_default_domain,
hashed_root_password,
minimal,
+ network_interface_pinning,
nic_matching,
specific_nic,
zfs,
@@ -149,6 +150,7 @@ mod tests {
fqdn_hostname_only,
ipv4_and_subnet_mask_33,
lvm_swapsize_greater_than_hdsize,
+ network_interface_pinning_overlong_interface_name,
no_fqdn_from_dhcp,
no_root_password_set,
short_password,
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/network_interface_pinning.json b/proxmox-auto-installer/tests/resources/parse_answer/network_interface_pinning.json
new file mode 100644
index 0000000..76723c8
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer/network_interface_pinning.json
@@ -0,0 +1,30 @@
+{
+ "autoreboot": 1,
+ "cidr": "192.168.1.114/24",
+ "country": "at",
+ "dns": "192.168.1.254",
+ "domain": "testinstall",
+ "filesys": "ext4",
+ "gateway": "192.168.1.1",
+ "hdsize": 223.57088470458984,
+ "existing_storage_auto_rename": 1,
+ "hostname": "pveauto",
+ "keymap": "de",
+ "mailto": "mail@no.invalid",
+ "mngmt_nic": "mgmt",
+ "network_interface_pin_map": {
+ "1c:34:da:5c:5e:24": "nic2",
+ "1c:34:da:5c:5e:25": "nic3",
+ "24:8a:07:1e:05:bc": "lan0",
+ "24:8a:07:1e:05:bd": "lan1",
+ "5a:47:32:dd:c7:47": "nic8",
+ "a0:36:9f:0a:b3:82": "nic6",
+ "a0:36:9f:0a:b3:83": "nic7",
+ "b4:2e:99:ac:ad:b4": "mgmt",
+ "b4:2e:99:ac:ad:b5": "nic1"
+ },
+ "root_password": { "plain": "12345678" },
+ "target_hd": "/dev/sda",
+ "timezone": "Europe/Vienna",
+ "first_boot": { "enabled": 0 }
+}
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/network_interface_pinning.toml b/proxmox-auto-installer/tests/resources/parse_answer/network_interface_pinning.toml
new file mode 100644
index 0000000..d9a2110
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer/network_interface_pinning.toml
@@ -0,0 +1,22 @@
+[global]
+keyboard = "de"
+country = "at"
+fqdn = "pveauto.testinstall"
+mailto = "mail@no.invalid"
+timezone = "Europe/Vienna"
+root-password = "12345678"
+
+[network]
+source = "from-dhcp"
+
+[network.interface-name-pinning]
+enabled = true
+
+[network.interface-name-pinning.mapping]
+"24:8a:07:1e:05:bc" = "lan0"
+"24:8a:07:1e:05:bd" = "lan1"
+"b4:2e:99:ac:ad:b4" = "mgmt"
+
+[disk-setup]
+filesystem = "ext4"
+disk-list = ["sda"]
diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/network_interface_pinning_overlong_interface_name.json b/proxmox-auto-installer/tests/resources/parse_answer_fail/network_interface_pinning_overlong_interface_name.json
new file mode 100644
index 0000000..af4ed79
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/network_interface_pinning_overlong_interface_name.json
@@ -0,0 +1,3 @@
+{
+ "parse-error": "error parsing answer.toml: interface name 'waytoolonginterfacename' for 'ab:cd:ef:12:34:56' cannot be longer than 15 characters"
+}
diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/network_interface_pinning_overlong_interface_name.toml b/proxmox-auto-installer/tests/resources/parse_answer_fail/network_interface_pinning_overlong_interface_name.toml
new file mode 100644
index 0000000..e82b47d
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/network_interface_pinning_overlong_interface_name.toml
@@ -0,0 +1,18 @@
+[global]
+keyboard = "de"
+country = "at"
+fqdn = "pveauto.fail.testinstall"
+mailto = "mail@no.invalid"
+timezone = "Europe/Vienna"
+root-password = "12345678"
+
+[network]
+source = "from-dhcp"
+interface-name-pinning.enabled = true
+
+[network.interface-name-pinning.mapping]
+"ab:cd:ef:12:34:56" = "waytoolonginterfacename"
+
+[disk-setup]
+filesystem = "ext4"
+disk-list = ["sda"]
--
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] 21+ messages in thread
* [pve-devel] [PATCH installer v3 08/15] assistant: verify network settings in `validate-answer` subcommand
2025-11-11 13:59 [pve-devel] [PATCH installer v3 00/15] support network interface name pinning Christoph Heiss
` (6 preceding siblings ...)
2025-11-11 13:59 ` [pve-devel] [PATCH installer v3 07/15] auto: add support for pinning network interface names Christoph Heiss
@ 2025-11-11 13:59 ` Christoph Heiss
2025-11-11 13:59 ` [pve-devel] [PATCH installer v3 09/15] post-hook: avoid redundant Option<bool> for (de-)serialization Christoph Heiss
` (9 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Christoph Heiss @ 2025-11-11 13:59 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v2 -> v3:
* no changes
Changes v1 -> v2:
* adapted to verify_network_settings() API change
proxmox-auto-install-assistant/src/main.rs | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/proxmox-auto-install-assistant/src/main.rs b/proxmox-auto-install-assistant/src/main.rs
index e0f2435..af5b703 100644
--- a/proxmox-auto-install-assistant/src/main.rs
+++ b/proxmox-auto-install-assistant/src/main.rs
@@ -23,7 +23,7 @@ use proxmox_auto_installer::{
AutoInstSettings, FetchAnswerFrom, HttpOptions, default_partition_label,
get_matched_udev_indexes, get_nic_list, get_single_udev_index, verify_disks_settings,
verify_email_and_root_password_settings, verify_first_boot_settings,
- verify_locale_settings,
+ verify_locale_settings, verify_network_settings,
},
};
use proxmox_installer_common::{FIRST_BOOT_EXEC_MAX_SIZE, FIRST_BOOT_EXEC_NAME, cli};
@@ -909,6 +909,7 @@ fn parse_answer(path: impl AsRef<Path> + fmt::Debug) -> Result<Answer> {
verify_disks_settings(&answer)?;
verify_first_boot_settings(&answer)?;
verify_email_and_root_password_settings(&answer)?;
+ verify_network_settings(&answer.network, None)?;
Ok(answer)
}
Err(err) => bail!("Error parsing answer file: {err}"),
--
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] 21+ messages in thread
* [pve-devel] [PATCH installer v3 09/15] post-hook: avoid redundant Option<bool> for (de-)serialization
2025-11-11 13:59 [pve-devel] [PATCH installer v3 00/15] support network interface name pinning Christoph Heiss
` (7 preceding siblings ...)
2025-11-11 13:59 ` [pve-devel] [PATCH installer v3 08/15] assistant: verify network settings in `validate-answer` subcommand Christoph Heiss
@ 2025-11-11 13:59 ` Christoph Heiss
2025-11-11 14:00 ` [pve-devel] [PATCH installer v3 10/15] post-hook: add network interface name and pinning status Christoph Heiss
` (8 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Christoph Heiss @ 2025-11-11 13:59 UTC (permalink / raw)
To: pve-devel
Instead, for the serialization case just skip it if the value is falsy,
for deserialization default-initialize it with `false`.
No functional changes.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v2 -> v3:
* no changes
Changes v1 -> v2:
* no changes
proxmox-installer-common/src/setup.rs | 12 ++----------
proxmox-post-hook/src/main.rs | 27 +++++++++++++++------------
2 files changed, 17 insertions(+), 22 deletions(-)
diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs
index 3e99576..1a584ba 100644
--- a/proxmox-installer-common/src/setup.rs
+++ b/proxmox-installer-common/src/setup.rs
@@ -275,14 +275,6 @@ where
Ok(val != 0)
}
-fn deserialize_bool_from_int_maybe<'de, D>(deserializer: D) -> Result<Option<bool>, D::Error>
-where
- D: Deserializer<'de>,
-{
- let val: Option<u32> = Deserialize::deserialize(deserializer)?;
- Ok(val.map(|v| v != 0))
-}
-
fn deserialize_cczones_map<'de, D>(
deserializer: D,
) -> Result<HashMap<String, Vec<String>>, D::Error>
@@ -389,8 +381,8 @@ pub struct RuntimeInfo {
pub hvm_supported: bool,
/// Whether the system was booted with SecureBoot enabled
- #[serde(default, deserialize_with = "deserialize_bool_from_int_maybe")]
- pub secure_boot: Option<bool>,
+ #[serde(default, deserialize_with = "deserialize_bool_from_int")]
+ pub secure_boot: bool,
/// Default upper limit for the ZFS ARC size, in MiB.
pub default_zfs_arc_max: usize,
diff --git a/proxmox-post-hook/src/main.rs b/proxmox-post-hook/src/main.rs
index 3897c26..36fda65 100644
--- a/proxmox-post-hook/src/main.rs
+++ b/proxmox-post-hook/src/main.rs
@@ -44,8 +44,8 @@ struct BootInfo {
/// Whether the system is booted using UEFI or legacy BIOS.
mode: BootType,
/// Whether SecureBoot is enabled for the installation.
- #[serde(skip_serializing_if = "Option::is_none")]
- secureboot: Option<bool>,
+ #[serde(skip_serializing_if = "bool_is_false")]
+ secureboot: bool,
}
/// Holds all the public keys for the different algorithms available.
@@ -66,8 +66,8 @@ struct DiskInfo {
/// Size in bytes
size: usize,
/// Set to true if the disk is used for booting.
- #[serde(skip_serializing_if = "Option::is_none")]
- is_bootdisk: Option<bool>,
+ #[serde(skip_serializing_if = "bool_is_false")]
+ is_bootdisk: bool,
/// Properties about the device as given by udev.
udev_properties: UdevProperties,
}
@@ -83,12 +83,16 @@ struct NetworkInterfaceInfo {
address: Option<CidrAddress>,
/// Set to true if the interface is the chosen management interface during
/// installation.
- #[serde(skip_serializing_if = "Option::is_none")]
- is_management: Option<bool>,
+ #[serde(skip_serializing_if = "bool_is_false")]
+ is_management: bool,
/// Properties about the device as given by udev.
udev_properties: UdevProperties,
}
+fn bool_is_false(value: &bool) -> bool {
+ !value
+}
+
/// Information about the installed product itself.
#[derive(Serialize)]
#[serde(rename_all = "kebab-case")]
@@ -323,7 +327,8 @@ impl PostHookInfo {
let is_bootdisk = config
.target_hd
.as_ref()
- .and_then(|hd| (*hd == disk.path).then_some(true));
+ .map(|hd| *hd == disk.path)
+ .unwrap_or_default();
anyhow::Ok(DiskInfo {
size: (config.hdsize * (SIZE_GIB as f64)) as usize,
@@ -341,9 +346,7 @@ impl PostHookInfo {
.disks
.iter()
.flat_map(|disk| {
- let is_bootdisk = selected_disks_indices
- .contains(&&disk.index)
- .then_some(true);
+ let is_bootdisk = selected_disks_indices.contains(&&disk.index);
anyhow::Ok(DiskInfo {
size: (config.hdsize * (SIZE_GIB as f64)) as usize,
@@ -389,14 +392,14 @@ impl PostHookInfo {
anyhow::Ok(NetworkInterfaceInfo {
mac: nic.mac.clone(),
address: Some(config.cidr.clone()),
- is_management: Some(true),
+ is_management: true,
udev_properties,
})
} else {
anyhow::Ok(NetworkInterfaceInfo {
mac: nic.mac.clone(),
address: None,
- is_management: None,
+ is_management: false,
udev_properties,
})
}
--
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] 21+ messages in thread
* [pve-devel] [PATCH installer v3 10/15] post-hook: add network interface name and pinning status
2025-11-11 13:59 [pve-devel] [PATCH installer v3 00/15] support network interface name pinning Christoph Heiss
` (8 preceding siblings ...)
2025-11-11 13:59 ` [pve-devel] [PATCH installer v3 09/15] post-hook: avoid redundant Option<bool> for (de-)serialization Christoph Heiss
@ 2025-11-11 14:00 ` Christoph Heiss
2025-11-11 14:00 ` [pve-devel] [PATCH installer v3 11/15] tui: views: move network options view to own module Christoph Heiss
` (7 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Christoph Heiss @ 2025-11-11 14:00 UTC (permalink / raw)
To: pve-devel
Adds a new `name` and `is-pinned` attribute to each reported network interface in
the post-hook data.
Also increments the minor schema version by one, as it is a
backwards-compatible change.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v2 -> v3:
* no changes
Changes v1 -> v2:
* no changes
proxmox-post-hook/src/main.rs | 39 +++++++++++++++++++++--------------
1 file changed, 23 insertions(+), 16 deletions(-)
diff --git a/proxmox-post-hook/src/main.rs b/proxmox-post-hook/src/main.rs
index 36fda65..9989389 100644
--- a/proxmox-post-hook/src/main.rs
+++ b/proxmox-post-hook/src/main.rs
@@ -76,6 +76,8 @@ struct DiskInfo {
#[derive(Serialize)]
#[serde(rename_all = "kebab-case")]
struct NetworkInterfaceInfo {
+ /// Name of the interface
+ name: String,
/// MAC address of the interface
mac: String,
/// (Designated) IP address of the interface
@@ -85,6 +87,10 @@ struct NetworkInterfaceInfo {
/// installation.
#[serde(skip_serializing_if = "bool_is_false")]
is_management: bool,
+ /// Set to true if the network interface name was pinned based on the MAC
+ /// address during the installation.
+ #[serde(skip_serializing_if = "bool_is_false")]
+ is_pinned: bool,
/// Properties about the device as given by udev.
udev_properties: UdevProperties,
}
@@ -151,7 +157,7 @@ struct PostHookInfoSchema {
}
impl PostHookInfoSchema {
- const SCHEMA_VERSION: &str = "1.1";
+ const SCHEMA_VERSION: &str = "1.2";
}
impl Default for PostHookInfoSchema {
@@ -386,23 +392,24 @@ impl PostHookInfo {
})?
.clone();
- if config.mngmt_nic == nic.name {
+ let is_pinned = config.network_interface_pin_map.contains_key(&nic.mac);
+ let ifname = config
+ .network_interface_pin_map
+ .get(&nic.mac)
+ .unwrap_or(&nic.name);
+
+ let is_management = config.mngmt_nic == *ifname;
+
+ anyhow::Ok(NetworkInterfaceInfo {
+ name: ifname.clone(),
+ mac: nic.mac.clone(),
// Use the actual IP address from the low-level install config, as the runtime info
// contains the original IP address from DHCP.
- anyhow::Ok(NetworkInterfaceInfo {
- mac: nic.mac.clone(),
- address: Some(config.cidr.clone()),
- is_management: true,
- udev_properties,
- })
- } else {
- anyhow::Ok(NetworkInterfaceInfo {
- mac: nic.mac.clone(),
- address: None,
- is_management: false,
- udev_properties,
- })
- }
+ address: is_management.then_some(config.cidr.clone()),
+ is_management,
+ is_pinned,
+ udev_properties,
+ })
})
.collect())
}
--
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] 21+ messages in thread
* [pve-devel] [PATCH installer v3 11/15] tui: views: move network options view to own module
2025-11-11 13:59 [pve-devel] [PATCH installer v3 00/15] support network interface name pinning Christoph Heiss
` (9 preceding siblings ...)
2025-11-11 14:00 ` [pve-devel] [PATCH installer v3 10/15] post-hook: add network interface name and pinning status Christoph Heiss
@ 2025-11-11 14:00 ` Christoph Heiss
2025-11-11 14:00 ` [pve-devel] [PATCH installer v3 12/15] tui: views: form: allow attaching user-defined data to children Christoph Heiss
` (6 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Christoph Heiss @ 2025-11-11 14:00 UTC (permalink / raw)
To: pve-devel
In preparation for adding network interface name pinning, which will
introduce quite a bit more functionality.
No functional changes.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v2 -> v3:
* no changes
Changes v1 -> v2:
* no changes
proxmox-tui-installer/src/main.rs | 90 +---------------
proxmox-tui-installer/src/views/mod.rs | 3 +
proxmox-tui-installer/src/views/network.rs | 113 +++++++++++++++++++++
3 files changed, 121 insertions(+), 85 deletions(-)
create mode 100644 proxmox-tui-installer/src/views/network.rs
diff --git a/proxmox-tui-installer/src/main.rs b/proxmox-tui-installer/src/main.rs
index 3379e7c..66eb44d 100644
--- a/proxmox-tui-installer/src/main.rs
+++ b/proxmox-tui-installer/src/main.rs
@@ -1,6 +1,6 @@
#![forbid(unsafe_code)]
-use std::{collections::HashMap, env, net::IpAddr};
+use std::{collections::HashMap, env};
use cursive::{
Cursive, CursiveRunnable, ScreenId, View, XY,
@@ -9,7 +9,7 @@ use cursive::{
view::{Nameable, Offset, Resizable, ViewWrapper},
views::{
Button, Checkbox, Dialog, DummyView, EditView, Layer, LinearLayout, PaddedView, Panel,
- ResizedView, ScrollView, SelectView, StackView, TextView,
+ ResizedView, ScrollView, StackView, TextView,
},
};
@@ -20,7 +20,6 @@ use proxmox_installer_common::{
ROOT_PASSWORD_MIN_LENGTH,
options::{BootdiskOptions, NetworkOptions, TimezoneOptions, email_validate},
setup::{LocaleInfo, ProxmoxProduct, RuntimeInfo, SetupInfo, installer_setup},
- utils::{CidrAddress, Fqdn},
};
mod setup;
@@ -28,7 +27,7 @@ mod system;
mod views;
use views::{
- BootdiskOptionsView, CidrAddressEditView, FormView, InstallProgressView, TableView,
+ BootdiskOptionsView, FormView, InstallProgressView, NetworkOptionsView, TableView,
TableViewItem, TimezoneOptionsView,
};
@@ -482,91 +481,12 @@ fn password_dialog(siv: &mut Cursive) -> InstallerView {
fn network_dialog(siv: &mut Cursive) -> InstallerView {
let state = siv.user_data::<InstallerState>().unwrap();
let options = &state.options.network;
- let ifaces = state.runtime_info.network.interfaces.values();
- let ifnames = ifaces
- .clone()
- .map(|iface| (iface.render(), iface.name.clone()));
- let mut ifaces_selection = SelectView::new().popup().with_all(ifnames.clone());
-
- // sort first to always have stable view
- ifaces_selection.sort();
- let selected = ifaces_selection
- .iter()
- .position(|(_label, iface)| *iface == options.ifname)
- .unwrap_or(ifaces.len() - 1);
-
- ifaces_selection.set_selection(selected);
-
- let inner = FormView::new()
- .child("Management interface", ifaces_selection)
- .child(
- "Hostname (FQDN)",
- EditView::new().content(options.fqdn.to_string()),
- )
- .child(
- "IP address (CIDR)",
- CidrAddressEditView::new().content(options.address.clone()),
- )
- .child(
- "Gateway address",
- EditView::new().content(options.gateway.to_string()),
- )
- .child(
- "DNS server address",
- EditView::new().content(options.dns_server.to_string()),
- )
- .with_name("network-options");
InstallerView::new(
state,
- inner,
+ NetworkOptionsView::new(options, &state.runtime_info.network).with_name("network-options"),
Box::new(|siv| {
- let options = siv.call_on_name("network-options", |view: &mut FormView| {
- let ifname = view
- .get_value::<SelectView, _>(0)
- .ok_or("failed to retrieve management interface name")?;
-
- let fqdn = view
- .get_value::<EditView, _>(1)
- .ok_or("failed to retrieve host FQDN")?
- .parse::<Fqdn>()
- .map_err(|err| format!("hostname does not look valid:\n\n{err}"))?;
-
- let address = view
- .get_value::<CidrAddressEditView, _>(2)
- .ok_or("failed to retrieve host address".to_string())
- .and_then(|(ip_addr, mask)| {
- CidrAddress::new(ip_addr, mask).map_err(|err| err.to_string())
- })?;
-
- let gateway = view
- .get_value::<EditView, _>(3)
- .ok_or("failed to retrieve gateway address")?
- .parse::<IpAddr>()
- .map_err(|err| err.to_string())?;
-
- let dns_server = view
- .get_value::<EditView, _>(4)
- .ok_or("failed to retrieve DNS server address")?
- .parse::<IpAddr>()
- .map_err(|err| err.to_string())?;
-
- if address.addr().is_ipv4() != gateway.is_ipv4() {
- Err("host and gateway IP address version must not differ".to_owned())
- } else if address.addr().is_ipv4() != dns_server.is_ipv4() {
- Err("host and DNS IP address version must not differ".to_owned())
- } else if fqdn.to_string().ends_with(".invalid") {
- Err("hostname does not look valid".to_owned())
- } else {
- Ok(NetworkOptions {
- ifname,
- fqdn,
- address,
- gateway,
- dns_server,
- })
- }
- });
+ let options = siv.call_on_name("network-options", NetworkOptionsView::get_values);
match options {
Some(Ok(options)) => {
diff --git a/proxmox-tui-installer/src/views/mod.rs b/proxmox-tui-installer/src/views/mod.rs
index 3417191..3a113da 100644
--- a/proxmox-tui-installer/src/views/mod.rs
+++ b/proxmox-tui-installer/src/views/mod.rs
@@ -16,6 +16,9 @@ pub use bootdisk::*;
mod install_progress;
pub use install_progress::*;
+mod network;
+pub use network::*;
+
mod tabbed_view;
pub use tabbed_view::*;
diff --git a/proxmox-tui-installer/src/views/network.rs b/proxmox-tui-installer/src/views/network.rs
new file mode 100644
index 0000000..5e3e258
--- /dev/null
+++ b/proxmox-tui-installer/src/views/network.rs
@@ -0,0 +1,113 @@
+use std::net::IpAddr;
+
+use cursive::{
+ view::ViewWrapper,
+ views::{EditView, SelectView},
+};
+
+use super::{CidrAddressEditView, FormView};
+use proxmox_installer_common::{
+ options::NetworkOptions,
+ setup::NetworkInfo,
+ utils::{CidrAddress, Fqdn},
+};
+
+pub struct NetworkOptionsView {
+ view: FormView,
+}
+
+impl NetworkOptionsView {
+ pub fn new(options: &NetworkOptions, network_info: &NetworkInfo) -> Self {
+ let ifaces = network_info.interfaces.values();
+ let ifnames = ifaces
+ .clone()
+ .map(|iface| (iface.render(), iface.name.clone()));
+ let mut ifaces_selection = SelectView::new().popup().with_all(ifnames.clone());
+
+ // sort first to always have stable view
+ ifaces_selection.sort();
+ let selected = ifaces_selection
+ .iter()
+ .position(|(_label, iface)| *iface == options.ifname)
+ .unwrap_or(ifaces.len() - 1);
+
+ ifaces_selection.set_selection(selected);
+
+ let view = FormView::new()
+ .child("Management interface", ifaces_selection)
+ .child(
+ "Hostname (FQDN)",
+ EditView::new().content(options.fqdn.to_string()),
+ )
+ .child(
+ "IP address (CIDR)",
+ CidrAddressEditView::new().content(options.address.clone()),
+ )
+ .child(
+ "Gateway address",
+ EditView::new().content(options.gateway.to_string()),
+ )
+ .child(
+ "DNS server address",
+ EditView::new().content(options.dns_server.to_string()),
+ );
+
+ Self { view }
+ }
+
+ pub fn get_values(&mut self) -> Result<NetworkOptions, String> {
+ let ifname = self
+ .view
+ .get_value::<SelectView, _>(0)
+ .ok_or("failed to retrieve management interface name")?;
+
+ let fqdn = self
+ .view
+ .get_value::<EditView, _>(1)
+ .ok_or("failed to retrieve host FQDN")?
+ .parse::<Fqdn>()
+ .map_err(|err| format!("hostname does not look valid:\n\n{err}"))?;
+
+ let address = self
+ .view
+ .get_value::<CidrAddressEditView, _>(2)
+ .ok_or("failed to retrieve host address".to_string())
+ .and_then(|(ip_addr, mask)| {
+ CidrAddress::new(ip_addr, mask).map_err(|err| err.to_string())
+ })?;
+
+ let gateway = self
+ .view
+ .get_value::<EditView, _>(3)
+ .ok_or("failed to retrieve gateway address")?
+ .parse::<IpAddr>()
+ .map_err(|err| err.to_string())?;
+
+ let dns_server = self
+ .view
+ .get_value::<EditView, _>(4)
+ .ok_or("failed to retrieve DNS server address")?
+ .parse::<IpAddr>()
+ .map_err(|err| err.to_string())?;
+
+ if address.addr().is_ipv4() != gateway.is_ipv4() {
+ Err("host and gateway IP address version must not differ".to_owned())
+ } else if address.addr().is_ipv4() != dns_server.is_ipv4() {
+ Err("host and DNS IP address version must not differ".to_owned())
+ } else if fqdn.to_string().ends_with(".invalid") {
+ Err("hostname does not look valid".to_owned())
+ } else {
+ Ok(NetworkOptions {
+ ifname,
+ fqdn,
+ address,
+ gateway,
+ dns_server,
+ })
+ }
+ }
+}
+
+impl ViewWrapper for NetworkOptionsView {
+ cursive::wrap_impl!(self.view: FormView);
+}
--
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] 21+ messages in thread
* [pve-devel] [PATCH installer v3 12/15] tui: views: form: allow attaching user-defined data to children
2025-11-11 13:59 [pve-devel] [PATCH installer v3 00/15] support network interface name pinning Christoph Heiss
` (10 preceding siblings ...)
2025-11-11 14:00 ` [pve-devel] [PATCH installer v3 11/15] tui: views: move network options view to own module Christoph Heiss
@ 2025-11-11 14:00 ` Christoph Heiss
2025-11-11 14:00 ` [pve-devel] [PATCH installer v3 13/15] tui: add support for pinning network interface names Christoph Heiss
` (5 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Christoph Heiss @ 2025-11-11 14:00 UTC (permalink / raw)
To: pve-devel
Allows storing (typesafe) arbitrary data together with each child.
No functional changes for existing code.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v2 -> v3:
* no changes
Changes v1 -> v2:
* no changes
proxmox-tui-installer/src/main.rs | 2 +-
proxmox-tui-installer/src/views/bootdisk.rs | 6 ++--
proxmox-tui-installer/src/views/mod.rs | 33 +++++++++++++++++----
proxmox-tui-installer/src/views/network.rs | 2 +-
4 files changed, 33 insertions(+), 10 deletions(-)
diff --git a/proxmox-tui-installer/src/main.rs b/proxmox-tui-installer/src/main.rs
index 66eb44d..910add7 100644
--- a/proxmox-tui-installer/src/main.rs
+++ b/proxmox-tui-installer/src/main.rs
@@ -419,7 +419,7 @@ fn password_dialog(siv: &mut Cursive) -> InstallerView {
let state = siv.user_data::<InstallerState>().unwrap();
let options = &state.options.password;
- let inner = FormView::new()
+ let inner = FormView::<()>::new()
.child(
"Root password [at least 8 characters]",
EditView::new().secret(),
diff --git a/proxmox-tui-installer/src/views/bootdisk.rs b/proxmox-tui-installer/src/views/bootdisk.rs
index 7e3680b..5ec3e83 100644
--- a/proxmox-tui-installer/src/views/bootdisk.rs
+++ b/proxmox-tui-installer/src/views/bootdisk.rs
@@ -47,7 +47,7 @@ impl BootdiskOptionsView {
pub fn new(siv: &mut Cursive, runinfo: &RuntimeInfo, options: &BootdiskOptions) -> Self {
let advanced_options = Arc::new(Mutex::new(options.clone()));
- let bootdisk_form = FormView::new()
+ let bootdisk_form = FormView::<()>::new()
.child(
"Target harddisk",
target_bootdisk_selectview(
@@ -152,7 +152,7 @@ impl AdvancedBootdiskOptionsView {
let mut view = LinearLayout::vertical()
.child(DummyView.full_width())
- .child(FormView::new().child("Filesystem", fstype_select))
+ .child(FormView::<()>::new().child("Filesystem", fstype_select))
.child(DummyView.full_width());
// Create the appropriate (inner) advanced options view
@@ -493,7 +493,7 @@ impl<T: View> MultiDiskOptionsView<T> {
selectable_disks.push(("-- do not use --".to_owned(), None));
- let mut disk_form = FormView::new();
+ let mut disk_form = FormView::<()>::new();
for (i, _) in avail_disks.iter().enumerate() {
disk_form.add_child(
&format!("Harddisk {i}"),
diff --git a/proxmox-tui-installer/src/views/mod.rs b/proxmox-tui-installer/src/views/mod.rs
index 3a113da..662840a 100644
--- a/proxmox-tui-installer/src/views/mod.rs
+++ b/proxmox-tui-installer/src/views/mod.rs
@@ -1,4 +1,4 @@
-use std::{net::IpAddr, str::FromStr, sync::Arc};
+use std::{collections::HashMap, net::IpAddr, str::FromStr, sync::Arc};
use cursive::{
Printer, Rect, Vec2, View,
@@ -415,17 +415,21 @@ impl FormViewGetValue<f64> for DiskSizeEditView {
}
}
-pub struct FormView {
+pub struct FormView<UDT = ()> {
view: LinearLayout,
+ user_data: HashMap<usize, UDT>,
}
-impl FormView {
+impl<UDT> FormView<UDT> {
pub fn new() -> Self {
let view = LinearLayout::horizontal()
.child(LinearLayout::vertical().full_width())
.child(LinearLayout::vertical().full_width());
- Self { view }
+ Self {
+ view,
+ user_data: HashMap::new(),
+ }
}
pub fn add_child(&mut self, label: &str, view: impl View) {
@@ -433,6 +437,12 @@ impl FormView {
self.add_to_column(1, view);
}
+ pub fn add_child_with_data(&mut self, label: &str, view: impl View, data: UDT) {
+ self.add_to_column(0, TextView::new(format!("{label}: ")).no_wrap());
+ self.add_to_column(1, view);
+ self.user_data.insert(self.len() - 1, data);
+ }
+
pub fn child(mut self, label: &str, view: impl View) -> Self {
self.add_child(label, view);
self
@@ -454,6 +464,19 @@ impl FormView {
.downcast_ref::<T>()
}
+ pub fn get_child_with_data<T: View>(&self, index: usize) -> Option<(&T, &UDT)> {
+ let view = self
+ .view
+ .get_child(1)?
+ .downcast_ref::<ResizedView<LinearLayout>>()?
+ .get_inner()
+ .get_child(index)?
+ .downcast_ref::<T>()?;
+
+ let data = self.user_data.get(&index)?;
+ Some((view, data))
+ }
+
pub fn get_child_mut<T: View>(&mut self, index: usize) -> Option<&mut T> {
self.view
.get_child_mut(1)?
@@ -510,7 +533,7 @@ impl FormView {
}
}
-impl ViewWrapper for FormView {
+impl<UDT: Send + Sync + 'static> ViewWrapper for FormView<UDT> {
cursive::wrap_impl!(self.view: LinearLayout);
fn wrap_important_area(&self, size: Vec2) -> Rect {
diff --git a/proxmox-tui-installer/src/views/network.rs b/proxmox-tui-installer/src/views/network.rs
index 5e3e258..960c25e 100644
--- a/proxmox-tui-installer/src/views/network.rs
+++ b/proxmox-tui-installer/src/views/network.rs
@@ -33,7 +33,7 @@ impl NetworkOptionsView {
ifaces_selection.set_selection(selected);
- let view = FormView::new()
+ let form = FormView::<()>::new()
.child("Management interface", ifaces_selection)
.child(
"Hostname (FQDN)",
--
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] 21+ messages in thread
* [pve-devel] [PATCH installer v3 13/15] tui: add support for pinning network interface names
2025-11-11 13:59 [pve-devel] [PATCH installer v3 00/15] support network interface name pinning Christoph Heiss
` (11 preceding siblings ...)
2025-11-11 14:00 ` [pve-devel] [PATCH installer v3 12/15] tui: views: form: allow attaching user-defined data to children Christoph Heiss
@ 2025-11-11 14:00 ` Christoph Heiss
2025-11-11 14:00 ` [pve-devel] [PATCH installer v3 14/15] ui: gtk3: allow passing of dialog parent window Christoph Heiss
` (4 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Christoph Heiss @ 2025-11-11 14:00 UTC (permalink / raw)
To: pve-devel
Adds an additional checkbox and option button in the network panel, the
latter triggering a dialog for setting custom names per network
interface present on the system.
Pinning is enabled by default.
Each pinned network interface name defaults to `nicN`, where N is the
pinned ID from the low-level installer.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v2 -> v3:
* no changes
Changes v1 -> v2:
* no changes
proxmox-installer-common/src/setup.rs | 10 +-
proxmox-tui-installer/src/main.rs | 13 +-
proxmox-tui-installer/src/setup.rs | 6 +-
proxmox-tui-installer/src/views/mod.rs | 5 +
proxmox-tui-installer/src/views/network.rs | 357 +++++++++++++++++++--
5 files changed, 353 insertions(+), 38 deletions(-)
diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs
index 1a584ba..c93ee30 100644
--- a/proxmox-installer-common/src/setup.rs
+++ b/proxmox-installer-common/src/setup.rs
@@ -436,7 +436,7 @@ pub struct Gateway {
pub gateway: IpAddr,
}
-#[derive(Clone, Deserialize, Serialize)]
+#[derive(Clone, Deserialize, Serialize, PartialEq, Eq, PartialOrd, Ord)]
#[serde(rename_all = "UPPERCASE")]
/// The current interface state as reported by the kernel.
pub enum InterfaceState {
@@ -483,7 +483,13 @@ pub struct Interface {
impl Interface {
// avoid display trait as this is not the string representation for a serializer
pub fn render(&self) -> String {
- format!("{} {} ({})", self.state.render(), self.name, self.mac)
+ format!(
+ "{} {} ({}, {})",
+ self.state.render(),
+ self.name,
+ self.mac,
+ self.driver
+ )
}
pub fn to_pinned(&self, options: &NetworkInterfacePinningOptions) -> Self {
diff --git a/proxmox-tui-installer/src/main.rs b/proxmox-tui-installer/src/main.rs
index 910add7..05e6a8e 100644
--- a/proxmox-tui-installer/src/main.rs
+++ b/proxmox-tui-installer/src/main.rs
@@ -18,7 +18,10 @@ use options::{InstallerOptions, PasswordOptions};
use proxmox_installer_common::{
ROOT_PASSWORD_MIN_LENGTH,
- options::{BootdiskOptions, NetworkOptions, TimezoneOptions, email_validate},
+ options::{
+ BootdiskOptions, NetworkInterfacePinningOptions, NetworkOptions, TimezoneOptions,
+ email_validate,
+ },
setup::{LocaleInfo, ProxmoxProduct, RuntimeInfo, SetupInfo, installer_setup},
};
mod setup;
@@ -167,7 +170,13 @@ fn main() {
bootdisk: BootdiskOptions::defaults_from(&runtime_info.disks[0]),
timezone: TimezoneOptions::defaults_from(&runtime_info, &locales),
password: Default::default(),
- network: NetworkOptions::defaults_from(&setup_info, &runtime_info.network, None),
+ network: NetworkOptions::defaults_from(
+ &setup_info,
+ &runtime_info.network,
+ None,
+ // We enable network interface pinning by default in the TUI
+ Some(&NetworkInterfacePinningOptions::default()),
+ ),
autoreboot: true,
},
setup_info,
diff --git a/proxmox-tui-installer/src/setup.rs b/proxmox-tui-installer/src/setup.rs
index d2ffb70..3ab1869 100644
--- a/proxmox-tui-installer/src/setup.rs
+++ b/proxmox-tui-installer/src/setup.rs
@@ -1,4 +1,4 @@
-use std::collections::{BTreeMap, HashMap};
+use std::collections::BTreeMap;
use crate::options::InstallerOptions;
use proxmox_installer_common::{
@@ -8,6 +8,8 @@ use proxmox_installer_common::{
impl From<InstallerOptions> for InstallConfig {
fn from(options: InstallerOptions) -> Self {
+ let pinning_opts = options.network.pinning_opts.as_ref();
+
let mut config = Self {
autoreboot: options.autoreboot as usize,
@@ -32,7 +34,7 @@ impl From<InstallerOptions> for InstallConfig {
root_ssh_keys: vec![],
mngmt_nic: options.network.ifname,
- network_interface_pin_map: HashMap::new(),
+ network_interface_pin_map: pinning_opts.map(|o| o.mapping.clone()).unwrap_or_default(),
// Safety: At this point, it is know that we have a valid FQDN, as
// this is set by the TUI network panel, which only lets the user
diff --git a/proxmox-tui-installer/src/views/mod.rs b/proxmox-tui-installer/src/views/mod.rs
index 662840a..35cf53a 100644
--- a/proxmox-tui-installer/src/views/mod.rs
+++ b/proxmox-tui-installer/src/views/mod.rs
@@ -437,6 +437,11 @@ impl<UDT> FormView<UDT> {
self.add_to_column(1, view);
}
+ pub fn add_child_with_custom_label(&mut self, label: &str, view: impl View) {
+ self.add_to_column(0, TextView::new(label).no_wrap());
+ self.add_to_column(1, view);
+ }
+
pub fn add_child_with_data(&mut self, label: &str, view: impl View, data: UDT) {
self.add_to_column(0, TextView::new(format!("{label}: ")).no_wrap());
self.add_to_column(1, view);
diff --git a/proxmox-tui-installer/src/views/network.rs b/proxmox-tui-installer/src/views/network.rs
index 960c25e..4388c7d 100644
--- a/proxmox-tui-installer/src/views/network.rs
+++ b/proxmox-tui-installer/src/views/network.rs
@@ -1,40 +1,82 @@
-use std::net::IpAddr;
-
use cursive::{
- view::ViewWrapper,
- views::{EditView, SelectView},
+ Cursive, View,
+ view::{Nameable, Resizable, ViewWrapper},
+ views::{
+ Button, Checkbox, Dialog, DummyView, EditView, LinearLayout, NamedView, ResizedView,
+ ScrollView, SelectView, TextView,
+ },
+};
+use std::{
+ collections::HashMap,
+ net::IpAddr,
+ sync::{Arc, Mutex},
};
use super::{CidrAddressEditView, FormView};
use proxmox_installer_common::{
- options::NetworkOptions,
- setup::NetworkInfo,
+ net::MAX_IFNAME_LEN,
+ options::{NetworkInterfacePinningOptions, NetworkOptions},
+ setup::{Interface, NetworkInfo},
utils::{CidrAddress, Fqdn},
};
+struct NetworkViewOptions {
+ selected_mac: String,
+ pinning_enabled: bool,
+ // For UI purposes, we want to always save the mapping, to save the state
+ // between toggling the checkbox
+ pinning_options: NetworkInterfacePinningOptions,
+}
+
+/// Convenience wrapper when needing to take a (interior-mutable) reference to
+/// `NetworkViewOptions`.
+type NetworkViewOptionsRef = Arc<Mutex<NetworkViewOptions>>;
+
+/// View for configuring anything related to network setup.
pub struct NetworkOptionsView {
- view: FormView,
+ view: LinearLayout,
+ options: NetworkViewOptionsRef,
}
impl NetworkOptionsView {
+ const PINNING_OPTIONS_BUTTON_NAME: &str = "network-pinning-options-button";
+ const MGMT_IFNAME_SELECTVIEW_NAME: &str = "network-management-ifname-selectview";
+
pub fn new(options: &NetworkOptions, network_info: &NetworkInfo) -> Self {
- let ifaces = network_info.interfaces.values();
- let ifnames = ifaces
- .clone()
- .map(|iface| (iface.render(), iface.name.clone()));
- let mut ifaces_selection = SelectView::new().popup().with_all(ifnames.clone());
+ let mut ifaces = network_info
+ .interfaces
+ .values()
+ .collect::<Vec<&Interface>>();
- // sort first to always have stable view
- ifaces_selection.sort();
- let selected = ifaces_selection
- .iter()
- .position(|(_label, iface)| *iface == options.ifname)
- .unwrap_or(ifaces.len() - 1);
+ // First, sort interfaces by their link state and then name
+ ifaces.sort_unstable_by_key(|x| (&x.state, &x.name));
- ifaces_selection.set_selection(selected);
+ let selected_mac = network_info
+ .interfaces
+ .get(&options.ifname)
+ .map(|iface| iface.mac.clone())
+ .unwrap_or_else(|| {
+ ifaces
+ .first()
+ .expect("at least one network interface")
+ .mac
+ .clone()
+ });
+
+ let options_ref = Arc::new(Mutex::new(NetworkViewOptions {
+ selected_mac,
+ pinning_enabled: options.pinning_opts.is_some(),
+ pinning_options: options.pinning_opts.clone().unwrap_or_default(),
+ }));
+
+ let iface_selection =
+ Self::build_mgmt_ifname_selectview(ifaces.clone(), options_ref.clone());
let form = FormView::<()>::new()
- .child("Management interface", ifaces_selection)
+ .child(
+ "Management interface",
+ iface_selection.with_name(Self::MGMT_IFNAME_SELECTVIEW_NAME),
+ )
.child(
"Hostname (FQDN)",
EditView::new().content(options.fqdn.to_string()),
@@ -52,44 +94,106 @@ impl NetworkOptionsView {
EditView::new().content(options.dns_server.to_string()),
);
- Self { view }
+ let pinning_checkbox = LinearLayout::horizontal()
+ .child(Checkbox::new().checked().on_change({
+ let ifaces = ifaces
+ .iter()
+ .map(|iface| (*iface).clone())
+ .collect::<Vec<Interface>>();
+ 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())
+ .child(DummyView.full_width())
+ .child(
+ Button::new("Pinning 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());
+
+ // Pre-compute the child's layout, since it might depend on the size. Without this,
+ // the view will be empty until focused.
+ // The screen size never changes in our case, so this is completely OK.
+ view.layout(siv.screen_size());
+
+ siv.add_layer(view);
+ }
+ })
+ .with_name(Self::PINNING_OPTIONS_BUTTON_NAME),
+ );
+
+ let view = LinearLayout::vertical()
+ .child(form)
+ .child(DummyView.full_width())
+ .child(pinning_checkbox);
+
+ Self {
+ view,
+ options: options_ref,
+ }
}
pub fn get_values(&mut self) -> Result<NetworkOptions, String> {
- let ifname = self
+ let form = self
.view
- .get_value::<SelectView, _>(0)
+ .get_child(0)
+ .and_then(|v| v.downcast_ref::<FormView>())
+ .ok_or("failed to retrieve network options form")?;
+
+ let iface = form
+ .get_value::<NamedView<SelectView<Interface>>, _>(0)
.ok_or("failed to retrieve management interface name")?;
- let fqdn = self
- .view
+ let fqdn = form
.get_value::<EditView, _>(1)
.ok_or("failed to retrieve host FQDN")?
.parse::<Fqdn>()
.map_err(|err| format!("hostname does not look valid:\n\n{err}"))?;
- let address = self
- .view
+ let address = form
.get_value::<CidrAddressEditView, _>(2)
.ok_or("failed to retrieve host address".to_string())
.and_then(|(ip_addr, mask)| {
CidrAddress::new(ip_addr, mask).map_err(|err| err.to_string())
})?;
- let gateway = self
- .view
+ let gateway = form
.get_value::<EditView, _>(3)
.ok_or("failed to retrieve gateway address")?
.parse::<IpAddr>()
.map_err(|err| err.to_string())?;
- let dns_server = self
- .view
+ let dns_server = form
.get_value::<EditView, _>(4)
.ok_or("failed to retrieve DNS server address")?
.parse::<IpAddr>()
.map_err(|err| err.to_string())?;
+ let pinning_opts = self
+ .options
+ .lock()
+ .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,
+ };
+
if address.addr().is_ipv4() != gateway.is_ipv4() {
Err("host and gateway IP address version must not differ".to_owned())
} else if address.addr().is_ipv4() != dns_server.is_ipv4() {
@@ -103,11 +207,200 @@ impl NetworkOptionsView {
address,
gateway,
dns_server,
+ pinning_opts,
})
}
}
+
+ fn custom_name_mapping_view(
+ network_info: &NetworkInfo,
+ options_ref: NetworkViewOptionsRef,
+ ) -> impl View {
+ const DIALOG_NAME: &str = "network-interface-name-pinning-dialog";
+
+ let mut interfaces = network_info
+ .interfaces
+ .values()
+ .collect::<Vec<&Interface>>();
+
+ 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::<Vec<Interface>>();
+ move |siv| {
+ let options = siv
+ .call_on_name(DIALOG_NAME, |view: &mut Dialog| {
+ view.get_content_mut()
+ .downcast_mut::<InterfacePinningOptionsView>()
+ .map(InterfacePinningOptionsView::get_values)
+ })
+ .flatten();
+
+ let options = match options {
+ Some(Ok(options)) => options,
+ Some(Err(err)) => {
+ siv.add_layer(Dialog::info(err));
+ 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))
+ }
+
+ fn refresh_ifname_selectview(
+ siv: &mut Cursive,
+ ifaces: &[Interface],
+ options_ref: NetworkViewOptionsRef,
+ ) {
+ siv.call_on_name(
+ Self::MGMT_IFNAME_SELECTVIEW_NAME,
+ |view: &mut SelectView<Interface>| {
+ *view = Self::build_mgmt_ifname_selectview(ifaces.iter().collect(), options_ref);
+ },
+ );
+ }
+
+ fn build_mgmt_ifname_selectview(
+ ifaces: Vec<&Interface>,
+ options_ref: NetworkViewOptionsRef,
+ ) -> SelectView<Interface> {
+ let options = options_ref.lock().expect("unpoisoned lock");
+
+ // Map all interfaces to a list of (human-readable interface name, [Interface]) pairs
+ let ifnames = ifaces
+ .iter()
+ .map(|iface| {
+ let iface = if options.pinning_enabled {
+ &iface.to_pinned(&options.pinning_options)
+ } else {
+ iface
+ };
+
+ (iface.render(), iface.clone())
+ })
+ .collect::<Vec<(String, Interface)>>();
+
+ 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.clone();
+ }
+ });
+
+ // Finally, (try to) select the current one
+ let selected = view
+ .iter()
+ .position(|(_label, iface)| iface.mac == options.selected_mac)
+ .unwrap_or(0); // we sort UP interfaces first, so select the first UP interface
+ //
+ view.set_selection(selected);
+
+ view
+ }
}
impl ViewWrapper for NetworkOptionsView {
- cursive::wrap_impl!(self.view: FormView);
+ cursive::wrap_impl!(self.view: LinearLayout);
+}
+
+struct InterfacePinningOptionsView {
+ view: ScrollView<NamedView<FormView<String>>>,
+}
+
+impl InterfacePinningOptionsView {
+ const FORM_NAME: &str = "network-interface-name-pinning-form";
+
+ fn new(interfaces: &[&Interface], options_ref: NetworkViewOptionsRef) -> Self {
+ let options = options_ref.lock().expect("unpoisoned lock");
+
+ let mut form = FormView::<String>::new();
+
+ for iface in interfaces {
+ let label = format!(
+ "{} ({}, {}, {})",
+ iface.mac, iface.name, iface.driver, iface.state
+ );
+
+ let view = LinearLayout::horizontal()
+ .child(DummyView.full_width()) // right align below form elements
+ .child(
+ EditView::new()
+ .content(iface.to_pinned(&options.pinning_options).name)
+ .max_content_width(MAX_IFNAME_LEN)
+ .fixed_width(MAX_IFNAME_LEN),
+ );
+
+ form.add_child_with_data(&label, view, iface.mac.clone());
+
+ if !iface.addresses.is_empty() {
+ for chunk in iface.addresses.chunks(2) {
+ let addrs = chunk
+ .iter()
+ .map(|v| v.to_string())
+ .collect::<Vec<String>>()
+ .join(", ");
+
+ form.add_child_with_custom_label(&format!(" {addrs}\n"), DummyView);
+ }
+ }
+ }
+
+ Self {
+ view: ScrollView::new(form.with_name(Self::FORM_NAME)),
+ }
+ }
+
+ fn get_values(&mut self) -> Result<NetworkInterfacePinningOptions, String> {
+ let form = self.view.get_inner_mut().get_mut();
+
+ let mut mapping = HashMap::new();
+
+ for i in 0..form.len() {
+ let (inner, mac) = match form.get_child_with_data::<LinearLayout>(i) {
+ Some(formdata) => formdata,
+ None => continue,
+ };
+
+ let name = inner
+ .get_child(1)
+ .and_then(|v| v.downcast_ref::<ResizedView<EditView>>())
+ .map(|v| v.get_inner().get_content())
+ .ok_or_else(|| format!("failed to retrieve pinning ID for interface {}", mac))?;
+
+ mapping.insert(mac.clone(), (*name).clone());
+ }
+
+ let opts = NetworkInterfacePinningOptions { mapping };
+ opts.verify().map_err(|err| err.to_string())?;
+
+ Ok(opts)
+ }
+}
+
+impl ViewWrapper for InterfacePinningOptionsView {
+ cursive::wrap_impl!(self.view: ScrollView<NamedView<FormView<String>>>);
}
--
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] 21+ messages in thread
* [pve-devel] [PATCH installer v3 14/15] ui: gtk3: allow passing of dialog parent window
2025-11-11 13:59 [pve-devel] [PATCH installer v3 00/15] support network interface name pinning Christoph Heiss
` (12 preceding siblings ...)
2025-11-11 14:00 ` [pve-devel] [PATCH installer v3 13/15] tui: add support for pinning network interface names Christoph Heiss
@ 2025-11-11 14:00 ` Christoph Heiss
2025-11-11 14:00 ` [pve-devel] [PATCH installer v3 15/15] gui: add support for pinning network interface names Christoph Heiss
` (3 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Christoph Heiss @ 2025-11-11 14:00 UTC (permalink / raw)
To: pve-devel
For GTK to properly z-order the dialog, i.e. so that it gets put on top,
the correct parent window must be specified.
By default this is the root window, but for some settings we open an
additional dialog window, in which these must be used as parent.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v2 -> v3:
* no changes
Changes v1 -> v2:
* new patch
Proxmox/UI.pm | 12 ++++++------
Proxmox/UI/Gtk3.pm | 12 ++++++------
2 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/Proxmox/UI.pm b/Proxmox/UI.pm
index 99988ab..55a78e4 100644
--- a/Proxmox/UI.pm
+++ b/Proxmox/UI.pm
@@ -48,13 +48,13 @@ my sub get_env {
}
sub message {
- my ($msg) = @_;
- get_ui()->message($msg);
+ my ($msg, $parentwindow) = @_;
+ get_ui()->message($msg, $parentwindow);
}
sub error {
- my ($msg) = @_;
- get_ui()->error($msg);
+ my ($msg, $parentwindow) = @_;
+ get_ui()->error($msg, $parentwindow);
}
sub finished {
@@ -63,8 +63,8 @@ sub finished {
}
sub prompt {
- my ($query) = @_;
- return get_ui()->prompt($query);
+ my ($query, $parentwindow) = @_;
+ return get_ui()->prompt($query, $parentwindow);
}
sub display_html {
diff --git a/Proxmox/UI/Gtk3.pm b/Proxmox/UI/Gtk3.pm
index 9af3d6a..82a4623 100644
--- a/Proxmox/UI/Gtk3.pm
+++ b/Proxmox/UI/Gtk3.pm
@@ -8,18 +8,18 @@ use Gtk3;
use base qw(Proxmox::UI::Base);
sub message {
- my ($self, $msg) = @_;
+ my ($self, $msg, $parentwindow) = @_;
- my $window = $self->{state}->{window};
+ my $window = $parentwindow // $self->{state}->{window};
my $dialog = Gtk3::MessageDialog->new($window, 'modal', 'info', 'ok', $msg);
$dialog->run();
$dialog->destroy();
}
sub error {
- my ($self, $msg) = @_;
+ my ($self, $msg, $parentwindow) = @_;
- my $window = $self->{state}->{window};
+ my $window = $parentwindow // $self->{state}->{window};
my $dialog = Gtk3::MessageDialog->new($window, 'modal', 'error', 'ok', $msg);
$dialog->run();
$dialog->destroy();
@@ -31,9 +31,9 @@ sub finished {
}
sub prompt {
- my ($self, $query) = @_;
+ my ($self, $query, $parentwindow) = @_;
- my $window = $self->{state}->{window};
+ my $window = $parentwindow // $self->{state}->{window};
my $dialog = Gtk3::MessageDialog->new($window, 'modal', 'question', 'ok-cancel', $query);
my $response = $dialog->run();
$dialog->destroy();
--
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] 21+ messages in thread
* [pve-devel] [PATCH installer v3 15/15] gui: add support for pinning network interface names
2025-11-11 13:59 [pve-devel] [PATCH installer v3 00/15] support network interface name pinning Christoph Heiss
` (13 preceding siblings ...)
2025-11-11 14:00 ` [pve-devel] [PATCH installer v3 14/15] ui: gtk3: allow passing of dialog parent window Christoph Heiss
@ 2025-11-11 14:00 ` Christoph Heiss
2025-11-11 16:49 ` Stoiko Ivanov
2025-11-11 15:04 ` [pve-devel] [PATCH installer v3 00/15] support network interface name pinning Thomas Lamprecht
` (2 subsequent siblings)
17 siblings, 1 reply; 21+ messages in thread
From: Christoph Heiss @ 2025-11-11 14:00 UTC (permalink / raw)
To: pve-devel
Adds an additional checkbox and option button in the network panel, the
latter triggering a dialog for setting custom names per network
interface present on the system.
Pinning is enabled by default.
Each pinned network interface name defaults to `nicN`, where N is the
pinned ID from the low-level installer.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v2 -> v3:
* moved pinning map validation into own subroutine in
Proxmox::Sys::Net
* added unit tests for the above, taken from the rust implementation
Changes v1 -> v2:
* added better validation for interface names, now according to our
`pve-iface` json schema
* implemented gtk-specific suggestions made by Maximilano
* run `make tidy`
* fixed error message dialog getting wrongly z-ordered
Proxmox/Sys/Net.pm | 62 ++++++++++-
proxinstall | 189 ++++++++++++++++++++++++++++++----
test/Makefile | 7 +-
test/validate-link-pin-map.pl | 42 ++++++++
4 files changed, 278 insertions(+), 22 deletions(-)
create mode 100755 test/validate-link-pin-map.pl
diff --git a/Proxmox/Sys/Net.pm b/Proxmox/Sys/Net.pm
index 2183d27..991f723 100644
--- a/Proxmox/Sys/Net.pm
+++ b/Proxmox/Sys/Net.pm
@@ -8,7 +8,21 @@ use Proxmox::Sys::Udev;
use JSON qw();
use base qw(Exporter);
-our @EXPORT_OK = qw(parse_ip_address parse_ip_mask parse_fqdn);
+our @EXPORT_OK = qw(
+ parse_ip_address
+ parse_ip_mask
+ parse_fqdn
+ validate_link_pin_map
+ 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 {
+ MAX_IFNAME_LEN => 15,
+ DEFAULT_PIN_PREFIX => 'nic',
+};
our $HOSTNAME_RE = "(?:[a-zA-Z0-9](?:[a-zA-Z0-9\-]{,61}?[a-zA-Z0-9])?)";
our $FQDN_RE = "(?:${HOSTNAME_RE}\\.)*${HOSTNAME_RE}";
@@ -310,4 +324,50 @@ sub parse_fqdn : prototype($) {
die "Hostname does not look like a fully qualified domain name\n";
}
+# Does some basic checks on a link name mapping.
+# Takes a hashref mapping mac => name.
+#
+# Includes checks for:
+# - empty interface names
+# - overlong interface names
+# - duplicate interface names
+# - only contains ASCII alphanumeric characters and underscore, as enforced by
+# our `pve-iface` json schema.
+sub validate_link_pin_map : prototype($) {
+ my ($mapping) = @_;
+ my $reverse_mapping = {};
+
+ while (my ($mac, $name) = each %$mapping) {
+ if (!defined($name) || $name eq '') {
+ die "interface name for '$mac' cannot be empty\n";
+ }
+
+ if (length($name) > MAX_IFNAME_LEN) {
+ die "interface name '$name' for '$mac' cannot be longer than "
+ . MAX_IFNAME_LEN
+ . " characters\n";
+ }
+
+ if ($name =~ m/^[0-9]+$/) {
+ die "interface name '$name' for '$mac' is invalid: "
+ . "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-zA-Z_][a-zA-Z0-9_]*$/) {
+ die "interface name '$name' for '$mac' is invalid: "
+ . "name must only consist of alphanumeric characters and underscores\n";
+ }
+
+ if ($reverse_mapping->{$name}) {
+ die "duplicate interface name mapping '$name' for: $mac, $reverse_mapping->{$name}\n";
+ }
+
+ $reverse_mapping->{$name} = $mac;
+ }
+}
+
1;
diff --git a/proxinstall b/proxinstall
index 5ba65fa..49dd796 100755
--- a/proxinstall
+++ b/proxinstall
@@ -37,7 +37,8 @@ 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);
+use Proxmox::Sys::Net
+ qw(parse_ip_address parse_ip_mask validate_link_pin_map MAX_IFNAME_LEN DEFAULT_PIN_PREFIX);
use Proxmox::UI;
my $step_number = 0; # Init number for global function list
@@ -340,11 +341,92 @@ my $create_basic_grid = sub {
return $grid;
};
+my sub create_network_interface_pin_view {
+ my ($done_cb) = @_;
+
+ my $dialog = Gtk3::Dialog->new();
+ $dialog->set_title('Interface Name Pinning Options');
+ $dialog->add_button('_OK', 'ok');
+
+ my $content = $dialog->get_content_area();
+
+ my $hbox = Gtk3::Box->new('horizontal', 0);
+ $content->pack_start($hbox, 1, 1, 5);
+
+ my $grid = Gtk3::Grid->new();
+ $grid->set_column_spacing(10);
+ $grid->set_row_spacing(10);
+
+ # make the list scrollable, in case there are lots of interfaces
+ my $scrolled_window = Gtk3::ScrolledWindow->new();
+ $scrolled_window->set_hexpand(1);
+ $scrolled_window->set_propagate_natural_height(1);
+
+ $scrolled_window->add($grid);
+ $scrolled_window->set_policy('never', 'automatic');
+ $scrolled_window->set_min_content_height(200);
+ $scrolled_window->set_margin_start(10);
+ $scrolled_window->set_margin_end(10);
+
+ $hbox->pack_start($scrolled_window, 1, 0, 5);
+
+ my $interfaces = Proxmox::Install::RunEnv::get()->{network}->{interfaces};
+ my $mapping = Proxmox::Install::Config::get_network_interface_pin_map();
+
+ my $inputs = {};
+ my $row = 0;
+ for my $ifname (sort keys $interfaces->%*) {
+ my $iface = $interfaces->{$ifname};
+
+ my $name = $mapping->{ $iface->{mac} };
+ my $label_text = "$iface->{mac} ($ifname, $iface->{driver}, $iface->{state})";
+
+ # if the interface has addresses assigned through DHCP, show them for
+ # reference
+ if (defined($iface->{addresses})) {
+ $label_text .=
+ "\n " . join(', ', map { "$_->{address}/$_->{prefix}" } @{ $iface->{addresses} });
+ }
+
+ my ($label, $input) = create_text_input($name, $label_text);
+ $label->set_xalign(0.);
+
+ $grid->attach($label, 0, $row, 1, 1);
+ $grid->attach($input, 1, $row, 1, 1);
+ $row++;
+
+ $inputs->{ $iface->{mac} } = $input;
+ }
+
+ $hbox->show_all();
+
+ $dialog->signal_connect(
+ response => sub {
+ my %new_mapping = map { $_ => $inputs->{$_}->get_text() } keys %$inputs;
+
+ eval { validate_link_pin_map(\%new_mapping); };
+ if ($@) {
+ Proxmox::UI::message($@, $dialog);
+ return;
+ }
+
+ Proxmox::Install::Config::set_network_interface_pin_map(\%new_mapping);
+ $dialog->destroy();
+ $done_cb->();
+ },
+ );
+
+ $dialog->present();
+}
+
sub create_ipconf_view {
cleanup_view();
Proxmox::UI::display_html('ipconf.htm');
+ my $run_env = Proxmox::Install::RunEnv::get();
+ my $ipconf = $run_env->{ipconf};
+
my $grid = &$create_basic_grid();
$grid->set_row_spacing(10);
$grid->set_column_spacing(10);
@@ -355,7 +437,7 @@ sub create_ipconf_view {
my ($cidr_label, $cidr_box, $ipconf_entry_addr, $ipconf_entry_mask) = create_cidr_inputs($cidr);
- my $device_model = Gtk3::ListStore->new('Glib::String', 'Glib::String');
+ my $device_model = Gtk3::ListStore->new('Glib::String', 'Glib::String', 'Glib::String');
my $device_cb = Gtk3::ComboBox->new_with_model($device_model);
$device_cb->set_active(0);
$device_cb->set_visible(1);
@@ -369,19 +451,62 @@ sub create_ipconf_view {
$device_cb->pack_start($cell, 0);
$device_cb->add_attribute($cell, 'text', 1);
- my $get_device_desc = sub {
- my $iface = shift;
- return "$iface->{name} - $iface->{mac} ($iface->{driver})";
+ my $refresh_device_cb = sub {
+ # clear all entries and re-add them with their new names
+ my $active = $device_cb->get_active();
+ $device_model->clear();
+
+ my $mapping = Proxmox::Install::Config::get_network_interface_pin_map();
+ my $i = 0;
+ for my $index (sort keys $ipconf->{ifaces}->%*) {
+ my $iface = $ipconf->{ifaces}->{$index};
+ my $iter = $device_model->append();
+
+ my $symbol = "$iface->{state}" eq "UP" ? "\x{25CF}" : ' ';
+ my $name =
+ $gtk_state->{network_pinning_enabled}
+ ? $mapping->{ $iface->{mac} }
+ : $iface->{name};
+
+ $device_model->set(
+ $iter,
+ 0 => $symbol,
+ 1 => "$name - $iface->{mac} ($iface->{driver})",
+ );
+ $i++;
+ }
+
+ # re-set the currently active entry to keep the users selection
+ $device_cb->set_active($active);
};
- my $run_env = Proxmox::Install::RunEnv::get();
- my $ipconf = $run_env->{ipconf};
+ my $name_pin_opts_button = Gtk3::Button->new('Options');
+ $name_pin_opts_button->set_sensitive($gtk_state->{network_pinning_enabled});
+ $name_pin_opts_button->signal_connect(
+ clicked => sub {
+ create_network_interface_pin_view($refresh_device_cb);
+ },
+ );
+
+ my $name_pin_checkbox = Gtk3::CheckButton->new('Pin network interface names');
+ $name_pin_checkbox->set_active($gtk_state->{network_pinning_enabled});
+ $name_pin_checkbox->signal_connect(
+ toggled => sub {
+ $name_pin_opts_button->set_sensitive(!!$name_pin_checkbox->get_active());
+ $gtk_state->{network_pinning_enabled} = !!$name_pin_checkbox->get_active();
+ $refresh_device_cb->();
+ },
+ );
my ($device_active_map, $device_active_reverse_map) = ({}, {});
my $device_change_handler = sub {
my $current = shift;
+ # happens during the clear + re-insertion of all interfaces after
+ # the pinning changed, can be safely ignored
+ return if $current->get_active() == -1;
+
my $new = $device_active_map->{ $current->get_active() };
my $iface = $ipconf->{ifaces}->{$new};
@@ -389,6 +514,7 @@ sub create_ipconf_view {
return if defined($selected) && $iface->{name} eq $selected;
Proxmox::Install::Config::set_mngmt_nic($iface->{name});
+
$ipconf_entry_addr->set_text($iface->{inet}->{addr} || $iface->{inet6}->{addr})
if $iface->{inet}->{addr} || $iface->{inet6}->{addr};
$ipconf_entry_mask->set_text($iface->{inet}->{prefix} || $iface->{inet6}->{prefix})
@@ -400,13 +526,6 @@ sub create_ipconf_view {
my $i = 0;
for my $index (sort keys $ipconf->{ifaces}->%*) {
my $iface = $ipconf->{ifaces}->{$index};
- my $iter = $device_model->append();
- my $symbol = "$iface->{state}" eq "UP" ? "\x{25CF}" : ' ';
- $device_model->set(
- $iter,
- 0 => $symbol,
- 1 => $get_device_desc->($iface),
- );
$device_active_map->{$i} = $index;
$device_active_reverse_map->{ $iface->{name} } = $i;
@@ -418,6 +537,9 @@ sub create_ipconf_view {
$i++;
}
+ # fill the combobox with entries
+ $refresh_device_cb->();
+
if (my $nic = Proxmox::Install::Config::get_mngmt_nic()) {
$initial_active_device_pos = $device_active_reverse_map->{$nic};
} else {
@@ -443,7 +565,7 @@ sub create_ipconf_view {
$label->set_xalign(1.0);
$grid->attach($label, 0, 0, 1, 1);
- $grid->attach($device_cb, 1, 0, 1, 1);
+ $grid->attach($device_cb, 1, 0, 2, 1);
my $fqdn = Proxmox::Install::Config::get_fqdn();
my $hostname = $run_env->{network}->{hostname} || $iso_env->{product};
@@ -452,17 +574,17 @@ sub create_ipconf_view {
my ($host_label, $hostentry) = create_text_input($fqdn, 'Hostname (FQDN)');
$grid->attach($host_label, 0, 1, 1, 1);
- $grid->attach($hostentry, 1, 1, 1, 1);
+ $grid->attach($hostentry, 1, 1, 2, 1);
$grid->attach($cidr_label, 0, 2, 1, 1);
- $grid->attach($cidr_box, 1, 2, 1, 1);
+ $grid->attach($cidr_box, 1, 2, 2, 1);
my $cfg_gateway = Proxmox::Install::Config::get_gateway();
my $gateway = $cfg_gateway // $ipconf->{gateway} || '192.168.100.1';
my ($gw_label, $ipconf_entry_gw) = create_text_input($gateway, 'Gateway');
$grid->attach($gw_label, 0, 3, 1, 1);
- $grid->attach($ipconf_entry_gw, 1, 3, 1, 1);
+ $grid->attach($ipconf_entry_gw, 1, 3, 2, 1);
my $cfg_dns = Proxmox::Install::Config::get_dns();
my $dnsserver = $cfg_dns // $ipconf->{dnsserver} || $gateway;
@@ -470,7 +592,10 @@ sub create_ipconf_view {
my ($dns_label, $ipconf_entry_dns) = create_text_input($dnsserver, 'DNS Server');
$grid->attach($dns_label, 0, 4, 1, 1);
- $grid->attach($ipconf_entry_dns, 1, 4, 1, 1);
+ $grid->attach($ipconf_entry_dns, 1, 4, 2, 1);
+
+ $grid->attach($name_pin_checkbox, 1, 5, 1, 1);
+ $grid->attach($name_pin_opts_button, 2, 5, 1, 1);
$gtk_state->{inbox}->show_all;
set_next(
@@ -538,6 +663,8 @@ sub create_ipconf_view {
}
Proxmox::Install::Config::set_dns($dns_ip);
+ $gtk_state->{network_pinning_enabled} = !!$name_pin_checkbox->get_active();
+
#print STDERR "TEST $ipaddress/$netmask $gateway_ip $dns_ip\n";
$step_number++;
@@ -573,6 +700,13 @@ sub create_ack_view {
my $country = Proxmox::Install::Config::get_country();
+ my $mngmt_nic = Proxmox::Install::Config::get_mngmt_nic();
+ my $iface = Proxmox::Install::RunEnv::get('network')->{interfaces}->{$mngmt_nic};
+
+ my $nic_mapping = Proxmox::Install::Config::get_network_interface_pin_map();
+ my $interface =
+ $gtk_state->{network_pinning_enabled} ? $nic_mapping->{ $iface->{mac} } : $iface->{name};
+
my %config_values = (
__target_hd__ => join(' | ', $target_hds->@*),
__target_fs__ => Proxmox::Install::Config::get_filesys(),
@@ -580,7 +714,7 @@ sub create_ack_view {
__timezone__ => Proxmox::Install::Config::get_timezone(),
__keymap__ => Proxmox::Install::Config::get_keymap(),
__mailto__ => Proxmox::Install::Config::get_mailto(),
- __interface__ => Proxmox::Install::Config::get_mngmt_nic(),
+ __interface__ => $interface,
__hostname__ => Proxmox::Install::Config::get_hostname(),
__cidr__ => Proxmox::Install::Config::get_cidr(),
__gateway__ => Proxmox::Install::Config::get_gateway(),
@@ -609,6 +743,12 @@ sub create_ack_view {
set_next(
undef,
sub {
+ # before starting the install, unset the name pinning map if it
+ # is disabled
+ if (!$gtk_state->{network_pinning_enabled}) {
+ Proxmox::Install::Config::set_network_interface_pin_map(undef);
+ }
+
$step_number++;
create_extract_view();
},
@@ -1775,6 +1915,15 @@ if (!$initial_error && (scalar keys $run_env->{ipconf}->{ifaces}->%* == 0)) {
$initial_error = 1;
Proxmox::UI::display_html("nonics.htm");
set_next("Reboot", sub { app_quit(0); });
+} else {
+ # we enable it by default for new installation
+ $gtk_state->{network_pinning_enabled} = 1;
+
+ # pre-fill the name mapping before starting
+ my %mapping = map {
+ $_->{mac} => DEFAULT_PIN_PREFIX . $_->{pinned_id}
+ } values $run_env->{network}->{interfaces}->%*;
+ Proxmox::Install::Config::set_network_interface_pin_map(\%mapping);
}
create_intro_view() if !$initial_error;
diff --git a/test/Makefile b/test/Makefile
index 70a05be..bfe7674 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -4,7 +4,8 @@ export PERLLIB=..
.PHONY: check
check: test-zfs-arc-max test-run-command test-parse-fqdn test-ui2-stdio \
- test-zfs-get-pool-list test-parse-kernel-cmdline
+ test-zfs-get-pool-list test-parse-kernel-cmdline \
+ test-validate-link-pin-map
.PHONY: test-zfs-arc-max
test-zfs-arc-max:
@@ -29,3 +30,7 @@ test-zfs-get-pool-list:
.PHONY: test-parse-kernel-cmdline
test-parse-kernel-cmdline:
./parse-kernel-cmdline.pl
+
+.PHONY: test-validate-link-pin-map
+test-validate-link-pin-map:
+ ./validate-link-pin-map.pl
diff --git a/test/validate-link-pin-map.pl b/test/validate-link-pin-map.pl
new file mode 100755
index 0000000..6386700
--- /dev/null
+++ b/test/validate-link-pin-map.pl
@@ -0,0 +1,42 @@
+#!/usr/bin/env perl
+
+use strict;
+use warnings;
+
+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");
+
+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",
+);
+
+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/);
+
+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",
+);
+
+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",
+);
+
+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",
+);
+
+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] 21+ messages in thread
* Re: [pve-devel] [PATCH installer v3 00/15] support network interface name pinning
2025-11-11 13:59 [pve-devel] [PATCH installer v3 00/15] support network interface name pinning Christoph Heiss
` (14 preceding siblings ...)
2025-11-11 14:00 ` [pve-devel] [PATCH installer v3 15/15] gui: add support for pinning network interface names Christoph Heiss
@ 2025-11-11 15:04 ` Thomas Lamprecht
2025-11-12 12:00 ` Christoph Heiss
2025-11-11 17:12 ` Stoiko Ivanov
2025-11-12 7:13 ` [pve-devel] applied: " Thomas Lamprecht
17 siblings, 1 reply; 21+ messages in thread
From: Thomas Lamprecht @ 2025-11-11 15:04 UTC (permalink / raw)
To: Proxmox VE development discussion, Christoph Heiss
Am 11.11.25 um 15:00 schrieb Christoph Heiss:
> This series adds support for pinning the names of network interfaces
> directly during the installation, for all of GUI, TUI and auto-installer.
thanks for this!
> There are also some smaller clean-ups and quality-of-life improvements
> interspersed - to keep the patches for each part of the installer
> together - in the series, which can also be applied
> separately/beforehand if wanted.
>
> Tested all combinations, i.e. for each of GUI, TUI and auto-installer,
> installed with pinning disabled (checking for regressions) and with
> pinning enabled at some custom interface names set.
What I noticed on smoke-testing the GTK UI: UX wise it would be nice to
detect duplicate names early, i.e., on closing the "option" window used
to configure pinned names manually.
But if it's just that, it's probably best done as follow-up.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [pve-devel] [PATCH installer v3 15/15] gui: add support for pinning network interface names
2025-11-11 14:00 ` [pve-devel] [PATCH installer v3 15/15] gui: add support for pinning network interface names Christoph Heiss
@ 2025-11-11 16:49 ` Stoiko Ivanov
0 siblings, 0 replies; 21+ messages in thread
From: Stoiko Ivanov @ 2025-11-11 16:49 UTC (permalink / raw)
To: Christoph Heiss; +Cc: Proxmox VE development discussion
one cosmetic nit (if even that) inline:
On Tue, 11 Nov 2025 15:00:05 +0100
Christoph Heiss <c.heiss@proxmox.com> wrote:
> Adds an additional checkbox and option button in the network panel, the
> latter triggering a dialog for setting custom names per network
> interface present on the system.
>
> Pinning is enabled by default.
>
> Each pinned network interface name defaults to `nicN`, where N is the
> pinned ID from the low-level installer.
>
> Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
> ---
> Changes v2 -> v3:
> * moved pinning map validation into own subroutine in
> Proxmox::Sys::Net
> * added unit tests for the above, taken from the rust implementation
>
> Changes v1 -> v2:
> * added better validation for interface names, now according to our
> `pve-iface` json schema
> * implemented gtk-specific suggestions made by Maximilano
> * run `make tidy`
> * fixed error message dialog getting wrongly z-ordered
>
> Proxmox/Sys/Net.pm | 62 ++++++++++-
> proxinstall | 189 ++++++++++++++++++++++++++++++----
> test/Makefile | 7 +-
> test/validate-link-pin-map.pl | 42 ++++++++
> 4 files changed, 278 insertions(+), 22 deletions(-)
> create mode 100755 test/validate-link-pin-map.pl
>
> diff --git a/Proxmox/Sys/Net.pm b/Proxmox/Sys/Net.pm
> index 2183d27..991f723 100644
> --- a/Proxmox/Sys/Net.pm
> +++ b/Proxmox/Sys/Net.pm
> @@ -8,7 +8,21 @@ use Proxmox::Sys::Udev;
> use JSON qw();
>
> use base qw(Exporter);
> -our @EXPORT_OK = qw(parse_ip_address parse_ip_mask parse_fqdn);
> +our @EXPORT_OK = qw(
> + parse_ip_address
> + parse_ip_mask
> + parse_fqdn
> + validate_link_pin_map
> + 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 {
> + MAX_IFNAME_LEN => 15,
> + DEFAULT_PIN_PREFIX => 'nic',
> +};
>
> our $HOSTNAME_RE = "(?:[a-zA-Z0-9](?:[a-zA-Z0-9\-]{,61}?[a-zA-Z0-9])?)";
> our $FQDN_RE = "(?:${HOSTNAME_RE}\\.)*${HOSTNAME_RE}";
> @@ -310,4 +324,50 @@ sub parse_fqdn : prototype($) {
> die "Hostname does not look like a fully qualified domain name\n";
> }
>
> +# Does some basic checks on a link name mapping.
> +# Takes a hashref mapping mac => name.
> +#
> +# Includes checks for:
> +# - empty interface names
> +# - overlong interface names
> +# - duplicate interface names
> +# - only contains ASCII alphanumeric characters and underscore, as enforced by
> +# our `pve-iface` json schema.
> +sub validate_link_pin_map : prototype($) {
> + my ($mapping) = @_;
> + my $reverse_mapping = {};
> +
> + while (my ($mac, $name) = each %$mapping) {
stumbled over this - as it's not too common in our perl code-base
later realized we use it in the installer quite a bit (since 2018:
2e33c3f ("implemented acknowledgement screen"))
then got a bit side-tracked - as of perl 5.36 this can also be solved with
a multi-value for loop - see:
https://perldoc.perl.org/functions/each
but it's ok as is!
> + if (!defined($name) || $name eq '') {
> + die "interface name for '$mac' cannot be empty\n";
> + }
> +
> + if (length($name) > MAX_IFNAME_LEN) {
> + die "interface name '$name' for '$mac' cannot be longer than "
> + . MAX_IFNAME_LEN
> + . " characters\n";
> + }
> +
> + if ($name =~ m/^[0-9]+$/) {
> + die "interface name '$name' for '$mac' is invalid: "
> + . "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-zA-Z_][a-zA-Z0-9_]*$/) {
> + die "interface name '$name' for '$mac' is invalid: "
> + . "name must only consist of alphanumeric characters and underscores\n";
> + }
> +
> + if ($reverse_mapping->{$name}) {
> + die "duplicate interface name mapping '$name' for: $mac, $reverse_mapping->{$name}\n";
> + }
> +
> + $reverse_mapping->{$name} = $mac;
> + }
> +}
> +
> 1;
> diff --git a/proxinstall b/proxinstall
> index 5ba65fa..49dd796 100755
> --- a/proxinstall
> +++ b/proxinstall
> @@ -37,7 +37,8 @@ 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);
> +use Proxmox::Sys::Net
> + qw(parse_ip_address parse_ip_mask validate_link_pin_map MAX_IFNAME_LEN DEFAULT_PIN_PREFIX);
> use Proxmox::UI;
>
> my $step_number = 0; # Init number for global function list
> @@ -340,11 +341,92 @@ my $create_basic_grid = sub {
> return $grid;
> };
>
> +my sub create_network_interface_pin_view {
> + my ($done_cb) = @_;
> +
> + my $dialog = Gtk3::Dialog->new();
> + $dialog->set_title('Interface Name Pinning Options');
> + $dialog->add_button('_OK', 'ok');
> +
> + my $content = $dialog->get_content_area();
> +
> + my $hbox = Gtk3::Box->new('horizontal', 0);
> + $content->pack_start($hbox, 1, 1, 5);
> +
> + my $grid = Gtk3::Grid->new();
> + $grid->set_column_spacing(10);
> + $grid->set_row_spacing(10);
> +
> + # make the list scrollable, in case there are lots of interfaces
> + my $scrolled_window = Gtk3::ScrolledWindow->new();
> + $scrolled_window->set_hexpand(1);
> + $scrolled_window->set_propagate_natural_height(1);
> +
> + $scrolled_window->add($grid);
> + $scrolled_window->set_policy('never', 'automatic');
> + $scrolled_window->set_min_content_height(200);
> + $scrolled_window->set_margin_start(10);
> + $scrolled_window->set_margin_end(10);
> +
> + $hbox->pack_start($scrolled_window, 1, 0, 5);
> +
> + my $interfaces = Proxmox::Install::RunEnv::get()->{network}->{interfaces};
> + my $mapping = Proxmox::Install::Config::get_network_interface_pin_map();
> +
> + my $inputs = {};
> + my $row = 0;
> + for my $ifname (sort keys $interfaces->%*) {
> + my $iface = $interfaces->{$ifname};
> +
> + my $name = $mapping->{ $iface->{mac} };
> + my $label_text = "$iface->{mac} ($ifname, $iface->{driver}, $iface->{state})";
> +
> + # if the interface has addresses assigned through DHCP, show them for
> + # reference
> + if (defined($iface->{addresses})) {
> + $label_text .=
> + "\n " . join(', ', map { "$_->{address}/$_->{prefix}" } @{ $iface->{addresses} });
> + }
> +
> + my ($label, $input) = create_text_input($name, $label_text);
> + $label->set_xalign(0.);
> +
> + $grid->attach($label, 0, $row, 1, 1);
> + $grid->attach($input, 1, $row, 1, 1);
> + $row++;
> +
> + $inputs->{ $iface->{mac} } = $input;
> + }
> +
> + $hbox->show_all();
> +
> + $dialog->signal_connect(
> + response => sub {
> + my %new_mapping = map { $_ => $inputs->{$_}->get_text() } keys %$inputs;
> +
> + eval { validate_link_pin_map(\%new_mapping); };
> + if ($@) {
> + Proxmox::UI::message($@, $dialog);
> + return;
> + }
> +
> + Proxmox::Install::Config::set_network_interface_pin_map(\%new_mapping);
> + $dialog->destroy();
> + $done_cb->();
> + },
> + );
> +
> + $dialog->present();
> +}
> +
> sub create_ipconf_view {
>
> cleanup_view();
> Proxmox::UI::display_html('ipconf.htm');
>
> + my $run_env = Proxmox::Install::RunEnv::get();
> + my $ipconf = $run_env->{ipconf};
> +
> my $grid = &$create_basic_grid();
> $grid->set_row_spacing(10);
> $grid->set_column_spacing(10);
> @@ -355,7 +437,7 @@ sub create_ipconf_view {
>
> my ($cidr_label, $cidr_box, $ipconf_entry_addr, $ipconf_entry_mask) = create_cidr_inputs($cidr);
>
> - my $device_model = Gtk3::ListStore->new('Glib::String', 'Glib::String');
> + my $device_model = Gtk3::ListStore->new('Glib::String', 'Glib::String', 'Glib::String');
> my $device_cb = Gtk3::ComboBox->new_with_model($device_model);
> $device_cb->set_active(0);
> $device_cb->set_visible(1);
> @@ -369,19 +451,62 @@ sub create_ipconf_view {
> $device_cb->pack_start($cell, 0);
> $device_cb->add_attribute($cell, 'text', 1);
>
> - my $get_device_desc = sub {
> - my $iface = shift;
> - return "$iface->{name} - $iface->{mac} ($iface->{driver})";
> + my $refresh_device_cb = sub {
> + # clear all entries and re-add them with their new names
> + my $active = $device_cb->get_active();
> + $device_model->clear();
> +
> + my $mapping = Proxmox::Install::Config::get_network_interface_pin_map();
> + my $i = 0;
> + for my $index (sort keys $ipconf->{ifaces}->%*) {
> + my $iface = $ipconf->{ifaces}->{$index};
> + my $iter = $device_model->append();
> +
> + my $symbol = "$iface->{state}" eq "UP" ? "\x{25CF}" : ' ';
> + my $name =
> + $gtk_state->{network_pinning_enabled}
> + ? $mapping->{ $iface->{mac} }
> + : $iface->{name};
> +
> + $device_model->set(
> + $iter,
> + 0 => $symbol,
> + 1 => "$name - $iface->{mac} ($iface->{driver})",
> + );
> + $i++;
> + }
> +
> + # re-set the currently active entry to keep the users selection
> + $device_cb->set_active($active);
> };
>
> - my $run_env = Proxmox::Install::RunEnv::get();
> - my $ipconf = $run_env->{ipconf};
> + my $name_pin_opts_button = Gtk3::Button->new('Options');
> + $name_pin_opts_button->set_sensitive($gtk_state->{network_pinning_enabled});
> + $name_pin_opts_button->signal_connect(
> + clicked => sub {
> + create_network_interface_pin_view($refresh_device_cb);
> + },
> + );
> +
> + my $name_pin_checkbox = Gtk3::CheckButton->new('Pin network interface names');
> + $name_pin_checkbox->set_active($gtk_state->{network_pinning_enabled});
> + $name_pin_checkbox->signal_connect(
> + toggled => sub {
> + $name_pin_opts_button->set_sensitive(!!$name_pin_checkbox->get_active());
> + $gtk_state->{network_pinning_enabled} = !!$name_pin_checkbox->get_active();
> + $refresh_device_cb->();
> + },
> + );
>
> my ($device_active_map, $device_active_reverse_map) = ({}, {});
>
> my $device_change_handler = sub {
> my $current = shift;
>
> + # happens during the clear + re-insertion of all interfaces after
> + # the pinning changed, can be safely ignored
> + return if $current->get_active() == -1;
> +
> my $new = $device_active_map->{ $current->get_active() };
> my $iface = $ipconf->{ifaces}->{$new};
>
> @@ -389,6 +514,7 @@ sub create_ipconf_view {
> return if defined($selected) && $iface->{name} eq $selected;
>
> Proxmox::Install::Config::set_mngmt_nic($iface->{name});
> +
> $ipconf_entry_addr->set_text($iface->{inet}->{addr} || $iface->{inet6}->{addr})
> if $iface->{inet}->{addr} || $iface->{inet6}->{addr};
> $ipconf_entry_mask->set_text($iface->{inet}->{prefix} || $iface->{inet6}->{prefix})
> @@ -400,13 +526,6 @@ sub create_ipconf_view {
> my $i = 0;
> for my $index (sort keys $ipconf->{ifaces}->%*) {
> my $iface = $ipconf->{ifaces}->{$index};
> - my $iter = $device_model->append();
> - my $symbol = "$iface->{state}" eq "UP" ? "\x{25CF}" : ' ';
> - $device_model->set(
> - $iter,
> - 0 => $symbol,
> - 1 => $get_device_desc->($iface),
> - );
> $device_active_map->{$i} = $index;
> $device_active_reverse_map->{ $iface->{name} } = $i;
>
> @@ -418,6 +537,9 @@ sub create_ipconf_view {
> $i++;
> }
>
> + # fill the combobox with entries
> + $refresh_device_cb->();
> +
> if (my $nic = Proxmox::Install::Config::get_mngmt_nic()) {
> $initial_active_device_pos = $device_active_reverse_map->{$nic};
> } else {
> @@ -443,7 +565,7 @@ sub create_ipconf_view {
> $label->set_xalign(1.0);
>
> $grid->attach($label, 0, 0, 1, 1);
> - $grid->attach($device_cb, 1, 0, 1, 1);
> + $grid->attach($device_cb, 1, 0, 2, 1);
>
> my $fqdn = Proxmox::Install::Config::get_fqdn();
> my $hostname = $run_env->{network}->{hostname} || $iso_env->{product};
> @@ -452,17 +574,17 @@ sub create_ipconf_view {
>
> my ($host_label, $hostentry) = create_text_input($fqdn, 'Hostname (FQDN)');
> $grid->attach($host_label, 0, 1, 1, 1);
> - $grid->attach($hostentry, 1, 1, 1, 1);
> + $grid->attach($hostentry, 1, 1, 2, 1);
>
> $grid->attach($cidr_label, 0, 2, 1, 1);
> - $grid->attach($cidr_box, 1, 2, 1, 1);
> + $grid->attach($cidr_box, 1, 2, 2, 1);
>
> my $cfg_gateway = Proxmox::Install::Config::get_gateway();
> my $gateway = $cfg_gateway // $ipconf->{gateway} || '192.168.100.1';
>
> my ($gw_label, $ipconf_entry_gw) = create_text_input($gateway, 'Gateway');
> $grid->attach($gw_label, 0, 3, 1, 1);
> - $grid->attach($ipconf_entry_gw, 1, 3, 1, 1);
> + $grid->attach($ipconf_entry_gw, 1, 3, 2, 1);
>
> my $cfg_dns = Proxmox::Install::Config::get_dns();
> my $dnsserver = $cfg_dns // $ipconf->{dnsserver} || $gateway;
> @@ -470,7 +592,10 @@ sub create_ipconf_view {
> my ($dns_label, $ipconf_entry_dns) = create_text_input($dnsserver, 'DNS Server');
>
> $grid->attach($dns_label, 0, 4, 1, 1);
> - $grid->attach($ipconf_entry_dns, 1, 4, 1, 1);
> + $grid->attach($ipconf_entry_dns, 1, 4, 2, 1);
> +
> + $grid->attach($name_pin_checkbox, 1, 5, 1, 1);
> + $grid->attach($name_pin_opts_button, 2, 5, 1, 1);
>
> $gtk_state->{inbox}->show_all;
> set_next(
> @@ -538,6 +663,8 @@ sub create_ipconf_view {
> }
> Proxmox::Install::Config::set_dns($dns_ip);
>
> + $gtk_state->{network_pinning_enabled} = !!$name_pin_checkbox->get_active();
> +
> #print STDERR "TEST $ipaddress/$netmask $gateway_ip $dns_ip\n";
>
> $step_number++;
> @@ -573,6 +700,13 @@ sub create_ack_view {
>
> my $country = Proxmox::Install::Config::get_country();
>
> + my $mngmt_nic = Proxmox::Install::Config::get_mngmt_nic();
> + my $iface = Proxmox::Install::RunEnv::get('network')->{interfaces}->{$mngmt_nic};
> +
> + my $nic_mapping = Proxmox::Install::Config::get_network_interface_pin_map();
> + my $interface =
> + $gtk_state->{network_pinning_enabled} ? $nic_mapping->{ $iface->{mac} } : $iface->{name};
> +
> my %config_values = (
> __target_hd__ => join(' | ', $target_hds->@*),
> __target_fs__ => Proxmox::Install::Config::get_filesys(),
> @@ -580,7 +714,7 @@ sub create_ack_view {
> __timezone__ => Proxmox::Install::Config::get_timezone(),
> __keymap__ => Proxmox::Install::Config::get_keymap(),
> __mailto__ => Proxmox::Install::Config::get_mailto(),
> - __interface__ => Proxmox::Install::Config::get_mngmt_nic(),
> + __interface__ => $interface,
> __hostname__ => Proxmox::Install::Config::get_hostname(),
> __cidr__ => Proxmox::Install::Config::get_cidr(),
> __gateway__ => Proxmox::Install::Config::get_gateway(),
> @@ -609,6 +743,12 @@ sub create_ack_view {
> set_next(
> undef,
> sub {
> + # before starting the install, unset the name pinning map if it
> + # is disabled
> + if (!$gtk_state->{network_pinning_enabled}) {
> + Proxmox::Install::Config::set_network_interface_pin_map(undef);
> + }
> +
> $step_number++;
> create_extract_view();
> },
> @@ -1775,6 +1915,15 @@ if (!$initial_error && (scalar keys $run_env->{ipconf}->{ifaces}->%* == 0)) {
> $initial_error = 1;
> Proxmox::UI::display_html("nonics.htm");
> set_next("Reboot", sub { app_quit(0); });
> +} else {
> + # we enable it by default for new installation
> + $gtk_state->{network_pinning_enabled} = 1;
> +
> + # pre-fill the name mapping before starting
> + my %mapping = map {
> + $_->{mac} => DEFAULT_PIN_PREFIX . $_->{pinned_id}
> + } values $run_env->{network}->{interfaces}->%*;
> + Proxmox::Install::Config::set_network_interface_pin_map(\%mapping);
> }
>
> create_intro_view() if !$initial_error;
> diff --git a/test/Makefile b/test/Makefile
> index 70a05be..bfe7674 100644
> --- a/test/Makefile
> +++ b/test/Makefile
> @@ -4,7 +4,8 @@ export PERLLIB=..
>
> .PHONY: check
> check: test-zfs-arc-max test-run-command test-parse-fqdn test-ui2-stdio \
> - test-zfs-get-pool-list test-parse-kernel-cmdline
> + test-zfs-get-pool-list test-parse-kernel-cmdline \
> + test-validate-link-pin-map
>
> .PHONY: test-zfs-arc-max
> test-zfs-arc-max:
> @@ -29,3 +30,7 @@ test-zfs-get-pool-list:
> .PHONY: test-parse-kernel-cmdline
> test-parse-kernel-cmdline:
> ./parse-kernel-cmdline.pl
> +
> +.PHONY: test-validate-link-pin-map
> +test-validate-link-pin-map:
> + ./validate-link-pin-map.pl
> diff --git a/test/validate-link-pin-map.pl b/test/validate-link-pin-map.pl
> new file mode 100755
> index 0000000..6386700
> --- /dev/null
> +++ b/test/validate-link-pin-map.pl
> @@ -0,0 +1,42 @@
> +#!/usr/bin/env perl
> +
> +use strict;
> +use warnings;
> +
> +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");
> +
> +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",
> +);
> +
> +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/);
> +
> +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",
> +);
> +
> +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",
> +);
> +
> +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",
> +);
> +
> +done_testing();
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [pve-devel] [PATCH installer v3 00/15] support network interface name pinning
2025-11-11 13:59 [pve-devel] [PATCH installer v3 00/15] support network interface name pinning Christoph Heiss
` (15 preceding siblings ...)
2025-11-11 15:04 ` [pve-devel] [PATCH installer v3 00/15] support network interface name pinning Thomas Lamprecht
@ 2025-11-11 17:12 ` Stoiko Ivanov
2025-11-12 7:13 ` [pve-devel] applied: " Thomas Lamprecht
17 siblings, 0 replies; 21+ messages in thread
From: Stoiko Ivanov @ 2025-11-11 17:12 UTC (permalink / raw)
To: Christoph Heiss; +Cc: Proxmox VE development discussion
Thanks for the quick iteration and the unifying of the implementations for
the validations!
I ran a few smoke-tests:
* installed a server in our test-lab which had issues with 9.0 (due to
duplicate predictable names (pci-path-based) for a 100g broadcom nic and
a 1g broadcom nic...) - this went fine (I installed with
filter.ID_NET_NAME set to the unpinned name - and the autoinstaller
correctly configured the corresponding pinned nic-name in
/etc/network/interfaces)
* during a few smoke-tests in a VM I noticed that the following nic-names
were accepted (by both the tui and gui installers):
'_' , 'a' - from a quick look at:
https://git.proxmox.com/?p=pve-common.git;a=blob;f=src/PVE/JSONSchema.pm;h=c6e0f36898487b9584221693b8846195f1ef87dd;hb=HEAD#l677
I think both would be rejected - so maybe we could adapt this in a
follow-up (and still cap the maximal length here to 15)
In any case - the series as-is is a worthy addition!
Reviewed-by: Stoiko Ivanov <s.ivanov@proxmox.com>
Tested-by: Stoiko Ivanov <s.ivanov@proxmox.com>
On Tue, 11 Nov 2025 14:59:50 +0100
Christoph Heiss <c.heiss@proxmox.com> wrote:
> This series adds support for pinning the names of network interfaces
> directly during the installation, for all of GUI, TUI and auto-installer.
>
> There are also some smaller clean-ups and quality-of-life improvements
> interspersed - to keep the patches for each part of the installer
> together - in the series, which can also be applied
> separately/beforehand if wanted.
>
> Tested all combinations, i.e. for each of GUI, TUI and auto-installer,
> installed with pinning disabled (checking for regressions) and with
> pinning enabled at some custom interface names set.
>
> The auto-installer changes can be tested by specifying e.g.
>
> [network.interface-name-pinning]
> enabled = true
>
> [network.interface-name-pinning.mapping]
> "ab:cd:ef:12:34:56" = "mgmt"
> "12:34:56:ab:cd:ef" = "lan0"
>
> in the answer file.
>
> If pinning is enabled, interfaces which have no explicit mapping
> specified in the answer file will be assigned a pinned name based on
> their enumeration and the fixed `nic` prefix, i.e. `nic2` for the third
> NIC enumerated during boot.
>
> The hardcoded `nic` prefix could potentially be made configurable in the
> future, although given that the auto-installer provides information
> about all present network interfaces in the answer POST request anyway,
> assigning all network interfaces a custom name programmatically is no
> big deal.
>
> History
> =======
> v2: https://lore.proxmox.com/pve-devel/20251030110627.812398-1-c.heiss@proxmox.com/
> v1: https://lore.proxmox.com/pve-devel/20251014132207.1171073-1-c.heiss@proxmox.com/
>
> Notable changes v2 -> v3:
> * matched up pinning map validation for Rust & Perl (hopefully I
> caught all cases this time), also adding unit tests to the latter
> * improved above checks for both sides, mostly w.r.t. numeric checks
>
> Notable changes v1 -> v2:
> * improved interface name validation according to our `pve-iface` json
> schema
> * fixed gui error dialog getting wrongly z-ordered
> * implemented gtk machinery suggestion by Maximilano
>
> Diffstat
> ========
> Christoph Heiss (14):
> test: parse-kernel-cmdline: fix module import statement
> run env: network: add kernel driver name to network interface info
> common: utils: fix clippy warnings
> common: setup: simplify network address list serialization
> common: implement support for `network_interface_pin_map` config
> auto: add support for pinning network interface names
> assistant: verify network settings in `validate-answer` subcommand
> post-hook: avoid redundant Option<bool> for (de-)serialization
> post-hook: add network interface name and pinning status
> tui: views: move network options view to own module
> tui: views: form: allow attaching user-defined data to children
> tui: add support for pinning network interface names
> ui: gtk3: allow passing of dialog parent window
> gui: add support for pinning network interface names
>
> Thomas Lamprecht (1):
> install: add support for network interface name pinning
>
> Proxmox/Install.pm | 47 +-
> Proxmox/Install/Config.pm | 8 +
> Proxmox/Install/RunEnv.pm | 11 +
> Proxmox/Sys/Net.pm | 116 ++++-
> Proxmox/UI.pm | 12 +-
> Proxmox/UI/Gtk3.pm | 12 +-
> proxinstall | 189 +++++++-
> proxmox-auto-install-assistant/src/main.rs | 39 +-
> proxmox-auto-installer/src/answer.rs | 63 ++-
> .../src/bin/proxmox-auto-installer.rs | 5 +-
> proxmox-auto-installer/src/utils.rs | 62 ++-
> proxmox-auto-installer/tests/parse-answer.rs | 2 +
> .../network_interface_pinning.json | 30 ++
> .../network_interface_pinning.toml | 22 +
> ...n_from_dhcp_no_default_domain.run-env.json | 36 +-
> ...rface_pinning_overlong_interface_name.json | 3 +
> ...rface_pinning_overlong_interface_name.toml | 18 +
> .../no_fqdn_from_dhcp.run-env.json | 36 +-
> .../tests/resources/run-env-info.json | 38 +-
> .../src/fetch_plugins/http.rs | 5 +-
> .../src/fetch_plugins/partition.rs | 5 +-
> proxmox-installer-common/src/lib.rs | 5 +
> proxmox-installer-common/src/options.rs | 274 ++++++++++--
> proxmox-installer-common/src/setup.rs | 78 +++-
> proxmox-installer-common/src/utils.rs | 6 +-
> proxmox-post-hook/src/main.rs | 73 ++--
> proxmox-tui-installer/src/main.rs | 110 +----
> proxmox-tui-installer/src/setup.rs | 3 +
> proxmox-tui-installer/src/views/bootdisk.rs | 8 +-
> proxmox-tui-installer/src/views/mod.rs | 90 ++--
> proxmox-tui-installer/src/views/network.rs | 406 ++++++++++++++++++
> test/Makefile | 7 +-
> test/parse-kernel-cmdline.pl | 2 +-
> test/validate-link-pin-map.pl | 42 ++
> 34 files changed, 1534 insertions(+), 329 deletions(-)
> create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/network_interface_pinning.json
> create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/network_interface_pinning.toml
> create mode 100644 proxmox-auto-installer/tests/resources/parse_answer_fail/network_interface_pinning_overlong_interface_name.json
> create mode 100644 proxmox-auto-installer/tests/resources/parse_answer_fail/network_interface_pinning_overlong_interface_name.toml
> create mode 100644 proxmox-tui-installer/src/views/network.rs
> create mode 100755 test/validate-link-pin-map.pl
>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* [pve-devel] applied: [PATCH installer v3 00/15] support network interface name pinning
2025-11-11 13:59 [pve-devel] [PATCH installer v3 00/15] support network interface name pinning Christoph Heiss
` (16 preceding siblings ...)
2025-11-11 17:12 ` Stoiko Ivanov
@ 2025-11-12 7:13 ` Thomas Lamprecht
17 siblings, 0 replies; 21+ messages in thread
From: Thomas Lamprecht @ 2025-11-12 7:13 UTC (permalink / raw)
To: pve-devel, Christoph Heiss
On Tue, 11 Nov 2025 14:59:50 +0100, Christoph Heiss wrote:
> This series adds support for pinning the names of network interfaces
> directly during the installation, for all of GUI, TUI and auto-installer.
>
> There are also some smaller clean-ups and quality-of-life improvements
> interspersed - to keep the patches for each part of the installer
> together - in the series, which can also be applied
> separately/beforehand if wanted.
>
> [...]
Applied, with the commit message of the first patch reworked to clarify that
the problem was that the method wasn't exported and adapting patch 14/15 with
s/parentwindow/parent_window/ for legibility (I see no advantage in "gluing"
words together), thanks!
[01/15] test: parse-kernel-cmdline: fix module import statement
commit: af803841c1fc574f5bf941810dfb479addcc9359
[02/15] install: add support for network interface name pinning
commit: dbf15f2efc17ee2b6064c8c374b0bfc75a53f92a
[03/15] run env: network: add kernel driver name to network interface info
commit: 729d3fbb907c50d12bcc880599fef82009a6af10
[04/15] common: utils: fix clippy warnings
commit: 4f0eb2cb7fdaeb7c9956358ba6ddb08c727bb307
[05/15] common: setup: simplify network address list serialization
commit: c4f06bbc5ee8a1c0b90c332fc415a1736d5452db
[06/15] common: implement support for `network_interface_pin_map` config
commit: abe9c548a31b9e2d4c2a33ae7b7d1644815bb5ff
[07/15] auto: add support for pinning network interface names
commit: bb621a816ee7c54d87d539839bc0a6471ab16ac9
[08/15] assistant: verify network settings in `validate-answer` subcommand
commit: b3b59ee6e15c1986f286aeee2bbbe342bd16ef0d
[09/15] post-hook: avoid redundant Option<bool> for (de-)serialization
commit: 0ed4c015592258abeb63c9babbd4bb2488266e08
[10/15] post-hook: add network interface name and pinning status
commit: 9527a9a091a9a2f1606541bc1a325548a3895551
[11/15] tui: views: move network options view to own module
commit: f018da27c1de6bd45ced598a76c5a158d1b0cab7
[12/15] tui: views: form: allow attaching user-defined data to children
commit: 350f13ec8ef6537d836398db3c7cd487c0449293
[13/15] tui: add support for pinning network interface names
commit: 437abf01c8aed0c9a761b1fa6c534ed71d86064f
[14/15] ui: gtk3: allow passing of dialog parent window
commit: f1fde3de46a9a86f2ae63670183cd84f47b46e55
[15/15] gui: add support for pinning network interface names
commit: adb85e8e2a0ff44095cbb7316adadecf9df995e7
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [pve-devel] [PATCH installer v3 00/15] support network interface name pinning
2025-11-11 15:04 ` [pve-devel] [PATCH installer v3 00/15] support network interface name pinning Thomas Lamprecht
@ 2025-11-12 12:00 ` Christoph Heiss
0 siblings, 0 replies; 21+ messages in thread
From: Christoph Heiss @ 2025-11-12 12:00 UTC (permalink / raw)
To: Thomas Lamprecht; +Cc: Proxmox VE development discussion
On Tue Nov 11, 2025 at 4:04 PM CET, Thomas Lamprecht wrote:
> Am 11.11.25 um 15:00 schrieb Christoph Heiss:
>> [..]
>> Tested all combinations, i.e. for each of GUI, TUI and auto-installer,
>> installed with pinning disabled (checking for regressions) and with
>> pinning enabled at some custom interface names set.
> What I noticed on smoke-testing the GTK UI: UX wise it would be nice to
> detect duplicate names early, i.e., on closing the "option" window used
> to configure pinned names manually.
>
> But if it's just that, it's probably best done as follow-up.
Just quickly tested this again to be sure, but that is already working?
I.e. if I enter `nic` for two different interfaces and click either the
"OK" button or the close button of the dialog, both trigger the
validation and show an error message.
Or am I missing something?
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2025-11-12 12:00 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-11-11 13:59 [pve-devel] [PATCH installer v3 00/15] support network interface name pinning Christoph Heiss
2025-11-11 13:59 ` [pve-devel] [PATCH installer v3 01/15] test: parse-kernel-cmdline: fix module import statement Christoph Heiss
2025-11-11 13:59 ` [pve-devel] [PATCH installer v3 02/15] install: add support for network interface name pinning Christoph Heiss
2025-11-11 13:59 ` [pve-devel] [PATCH installer v3 03/15] run env: network: add kernel driver name to network interface info Christoph Heiss
2025-11-11 13:59 ` [pve-devel] [PATCH installer v3 04/15] common: utils: fix clippy warnings Christoph Heiss
2025-11-11 13:59 ` [pve-devel] [PATCH installer v3 05/15] common: setup: simplify network address list serialization Christoph Heiss
2025-11-11 13:59 ` [pve-devel] [PATCH installer v3 06/15] common: implement support for `network_interface_pin_map` config Christoph Heiss
2025-11-11 13:59 ` [pve-devel] [PATCH installer v3 07/15] auto: add support for pinning network interface names Christoph Heiss
2025-11-11 13:59 ` [pve-devel] [PATCH installer v3 08/15] assistant: verify network settings in `validate-answer` subcommand Christoph Heiss
2025-11-11 13:59 ` [pve-devel] [PATCH installer v3 09/15] post-hook: avoid redundant Option<bool> for (de-)serialization Christoph Heiss
2025-11-11 14:00 ` [pve-devel] [PATCH installer v3 10/15] post-hook: add network interface name and pinning status Christoph Heiss
2025-11-11 14:00 ` [pve-devel] [PATCH installer v3 11/15] tui: views: move network options view to own module Christoph Heiss
2025-11-11 14:00 ` [pve-devel] [PATCH installer v3 12/15] tui: views: form: allow attaching user-defined data to children Christoph Heiss
2025-11-11 14:00 ` [pve-devel] [PATCH installer v3 13/15] tui: add support for pinning network interface names Christoph Heiss
2025-11-11 14:00 ` [pve-devel] [PATCH installer v3 14/15] ui: gtk3: allow passing of dialog parent window Christoph Heiss
2025-11-11 14:00 ` [pve-devel] [PATCH installer v3 15/15] gui: add support for pinning network interface names Christoph Heiss
2025-11-11 16:49 ` Stoiko Ivanov
2025-11-11 15:04 ` [pve-devel] [PATCH installer v3 00/15] support network interface name pinning Thomas Lamprecht
2025-11-12 12:00 ` Christoph Heiss
2025-11-11 17:12 ` Stoiko Ivanov
2025-11-12 7:13 ` [pve-devel] applied: " Thomas Lamprecht
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox