From: Thomas Lamprecht <t.lamprecht@proxmox.com>
Date: Fri, 24 Sep 2021 10:29:46 +0000 (+0200)
Subject: inotify: network: improve "allow-hotplug" & "auto" interaction
X-Git-Url: https://git.proxmox.com/?p=pve-common.git;a=commitdiff_plain;h=0dc7fd7b50918368ea3bed603eb2bb9a5cbdc982

inotify: network: improve "allow-hotplug" & "auto" interaction

commit c86cfb8bbd9b505d06b580582297fa670561437b dropped allow-hotplug
from the primary interfaces file completely on write, but that breaks
setups that come from plain Debian.

Instead, as stop-gap measurement, transform "allow-hotplug" to auto
in the PVE controlled config.

That avoids conflict and improves installing PVE on top of plain
Debian, as the interface still comes up after the first reboot.

But it is not ideal auto is not the same as hotplug, so we need to
also track that difference in the future, but that needs some
adaptions in the API too (change autostart from boolean to
string+enum or so=

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---

diff --git a/src/PVE/INotify.pm b/src/PVE/INotify.pm
index 65fa25f..0d82a52 100644
--- a/src/PVE/INotify.pm
+++ b/src/PVE/INotify.pm
@@ -904,12 +904,16 @@ sub __read_etc_network_interfaces {
     SECTION: while (defined ($line = <$fh>)) {
 	chomp ($line);
 	next if $line =~ m/^\s*#/;
-	next if $line =~ m/^\s*(allow-hotplug)\s+(.*)$/;
 
 	if ($line =~ m/^\s*(allow-auto|auto|allow-ovs)\s+(.*)$/) {
 
 	    $ifaces->{$_}->{autostart} = 1 for split (/\s+/, $2);
 
+	} elsif ($line =~ m/^\s*(allow-hotplug)\s+(.*)$/) {
+
+	    # FIXME: handle those differently? auto makes it required on-boot, vs. best-effort
+	    $ifaces->{$_}->{autostart} = 1 for split (/\s+/, $2);
+
 	} elsif ($line =~ m/^\s*iface\s+(\S+)\s+(inet6?)\s+(\S+)\s*$/) {
 	    my $i = $1;
 	    my $family = $2;
diff --git a/test/etc_network_interfaces/base-allow-hotplug b/test/etc_network_interfaces/base-allow-hotplug
new file mode 100644
index 0000000..967aeab
--- /dev/null
+++ b/test/etc_network_interfaces/base-allow-hotplug
@@ -0,0 +1,17 @@
+# network interface settings; autogenerated
+# Please do NOT modify this file directly, unless you know what
+# you're doing.
+#
+# If you want to manage parts of the network configuration manually,
+# please utilize the 'source' or 'source-directory' directives to do
+# so.
+# PVE will preserve these directives, but will NOT read its network
+# configuration from sourced files, so do not attempt to move any of
+# the PVE managed interfaces into external files!
+
+auto lo
+iface lo inet loopback
+
+allow-hotplug ens18
+iface ens18 inet dhcp
+
diff --git a/test/etc_network_interfaces/base-auto-allow-hotplug b/test/etc_network_interfaces/base-auto-allow-hotplug
new file mode 100644
index 0000000..b3aae7f
--- /dev/null
+++ b/test/etc_network_interfaces/base-auto-allow-hotplug
@@ -0,0 +1,18 @@
+# network interface settings; autogenerated
+# Please do NOT modify this file directly, unless you know what
+# you're doing.
+#
+# If you want to manage parts of the network configuration manually,
+# please utilize the 'source' or 'source-directory' directives to do
+# so.
+# PVE will preserve these directives, but will NOT read its network
+# configuration from sourced files, so do not attempt to move any of
+# the PVE managed interfaces into external files!
+
+auto lo
+iface lo inet loopback
+
+auto ens18
+allow-hotplug ens18
+iface ens18 inet dhcp
+
diff --git a/test/etc_network_interfaces/t.base-auto-allow-hotplug.pl b/test/etc_network_interfaces/t.base-auto-allow-hotplug.pl
new file mode 100644
index 0000000..772da83
--- /dev/null
+++ b/test/etc_network_interfaces/t.base-auto-allow-hotplug.pl
@@ -0,0 +1,25 @@
+my $active_ifaces = ['lo', 'ens18', 'ens'];
+my $proc_net = load('proc_net_dev');
+$proc_net =~ s/eth0/ens18/;
+
+my $wanted = load('base-allow-hotplug');
+
+# parse the config
+r($wanted, $proc_net, $active_ifaces);
+
+$wanted =~ s/allow-hotplug ens18/auto ens18/; # FIXME: hack! rather we need to keep allow-hotplug!
+
+expect $wanted;
+
+# idempotency (save, re-parse, and re-check)
+r(w(), $proc_net, $active_ifaces);
+expect $wanted;
+
+# parse one with both, "auto" and "allow-hotplug"
+my $bad = load('base-auto-allow-hotplug');
+r($bad, $proc_net, $active_ifaces);
+
+# should drop the first occuring one of the conflicting options ("auto" currently)
+expect $wanted;
+
+1;