Re: [pve-devel] [PATCH installer 14/14] gui: add support for pinning network interface names

["Christoph Heiss" <c.heiss@proxmox.com>]

On Tue Oct 21, 2025 at 4:05 PM CEST, Michael Köppl wrote:
> 3 comments inline
>
> On Tue Oct 14, 2025 at 3:21 PM CEST, Christoph Heiss wrote:
[..]
>> +    $dialog->signal_connect(
>> +        response => sub {
>> +            my $new_mapping = {};
>> +            my $reverse_mapping = {};
>> +            foreach my $mac (keys %$inputs) {
>> +                my $name = $inputs->{$mac}->get_text();
>> +
>> +                if (!defined($name) || $name eq '') {
>> +                    Proxmox::UI::message("interface name mapping for $mac cannot be empty");
>> +                    $inputs->{$mac}->grab_focus();
>> +                    return;
>> +                }
>> +
>> +                if ($reverse_mapping->{$name}) {
>> +                    Proxmox::UI::message(
>> +                        "duplicate interface name mapping '$name' for: $mac, $reverse_mapping->{$name}"
>> +                    );
>> +                    $inputs->{$mac}->grab_focus();
>> +                    return;
>> +                }
>> +
>> +                if (length($name) > MAX_IFNAME_LEN) {
>> +                    Proxmox::UI::message(
>> +                        "interface name mapping '$name' for $mac cannot be longer than "
>> +                            . MAX_IFNAME_LEN
>> +                            . " characters");
>> +                    $inputs->{$mac}->grab_focus();
>> +                    return;
>> +                }
>
> same as for the TUI installer, imo the check for only-numeric interface
> names would make sense. Or is there a reason it was omitted?

No particular reason, I'll add it here too for v2.
IIRC that seems to just have slipped through, as the first draft was
with a fixed prefix, since our tooling didn't support fully custom names
back then.

[..]
>> -    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};
>
> This seems to need a `make tidy`?
>

I'll run it before sending v2, thanks :^)



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel