[pve-devel] [RFC PATCH 6/7] write_etc_network_interfaces: write options only once

[Wolfgang Bumiller]

Options such as bridge ports or ovs_* will now be written
out only for the first interface. If multiple protocol
families of a bridge contain bridge_ports lines they will be
merged into the first interface.
---
 src/PVE/INotify.pm | 51 ++++++++++++++++++++++++++++-----------------------
 1 file changed, 28 insertions(+), 23 deletions(-)

diff --git a/src/PVE/INotify.pm b/src/PVE/INotify.pm
index bc6c345..3d91aad 100644
--- a/src/PVE/INotify.pm
+++ b/src/PVE/INotify.pm
@@ -990,7 +990,7 @@ sub read_etc_network_interfaces {
 }
 
 sub __interface_to_string {
-    my ($iface, $d, $family) = @_;
+    my ($iface, $d, $family, $once) = @_;
 
     return '' if !($d && $d->{method});
 
@@ -1004,53 +1004,49 @@ sub __interface_to_string {
     $raw .= "\tgateway  " . $d->{"gateway$suffix"} . "\n" if $d->{"gateway$suffix"};
     $raw .= "\tbroadcast  " . $d->{"broadcast$suffix"} . "\n" if $d->{"broadcast$suffix"};
 
-    my $done = { type => 1, priority => 1, method => 1, active => 1, exists => 1,
-		 comments => 1, autostart => 1, options => 1,
-		 address => 1, netmask => 1, gateway => 1, broadcast => 1,
-		 method6=> 1, families => 1,
-		 address6 => 1, netmask6 => 1, gateway6 => 1, broadcast6 => 1 };
- 
-    if ($d->{type} eq 'bridge') {
+    if (scalar(keys %$once)) {
+	# type-specific options are all only printed once
+    } elsif ($d->{type} eq 'bridge') {
 
 	my @ports = split(/\n/, $d->{bridge_ports} || 'none');
 	$raw .= "\tbridge_ports $_\n" foreach @ports;
-	$done->{bridge_ports} = 1;
+	$once->{bridge_ports} = 1;
 
 	my $v = defined($d->{bridge_stp}) ? $d->{bridge_stp} : 'off';
 	$raw .= "\tbridge_stp $v\n";
-	$done->{bridge_stp} = 1;
+	$once->{bridge_stp} = 1;
 
 	$v = defined($d->{bridge_fd}) ? $d->{bridge_fd} : 0;
 	$raw .= "\tbridge_fd $v\n";
-	$done->{bridge_fd} = 1;
+	$once->{bridge_fd} = 1;
     
     } elsif ($d->{type} eq 'bond') {
 
 	my $slaves = $d->{slaves} || 'none';
 	$raw .= "\tslaves $slaves\n";
-	$done->{slaves} = 1;
+	$once->{slaves} = 1;
 
 	my $v = defined ($d->{'bond_miimon'}) ? $d->{'bond_miimon'} : 100;
 	$raw .= "\tbond_miimon $v\n";
-	$done->{'bond_miimon'} = 1;
+	$once->{'bond_miimon'} = 1;
 
 	$v = defined ($d->{'bond_mode'}) ? $d->{'bond_mode'} : 'balance-rr';
 	$raw .= "\tbond_mode $v\n";
-	$done->{'bond_mode'} = 1;
+	$once->{'bond_mode'} = 1;
 
 	if ($d->{'bond_mode'} && $d->{'bond_xmit_hash_policy'} &&
 	    ($d->{'bond_mode'} eq 'balance-xor' || $d->{'bond_mode'} eq '802.3ad')) {
 	    $raw .= "\tbond_xmit_hash_policy $d->{'bond_xmit_hash_policy'}\n";
 	}
-	$done->{'bond_xmit_hash_policy'} = 1;
+	$once->{'bond_xmit_hash_policy'} = 1;
 
     } elsif ($d->{type} eq 'OVSBridge') {
 
 	$raw .= "\tovs_type $d->{type}\n";
-	$done->{ovs_type} = 1;
+	$once->{ovs_type} = 1;
 
 	$raw .= "\tovs_ports $d->{ovs_ports}\n" if $d->{ovs_ports};
-	$done->{ovs_ports} = 1;
+	$once->{ovs_ports} = 1;
 
     } elsif ($d->{type} eq 'OVSPort' || $d->{type} eq 'OVSIntPort' ||
 	     $d->{type} eq 'OVSBond') {
@@ -1060,7 +1056,7 @@ sub __interface_to_string {
 	if (defined($d->{ovs_tag})) {
 	    &$set_ovs_option($d, tag => $d->{ovs_tag});
 	}
-	$done->{ovs_tag} = 1;
+	$once->{ovs_tag} = 1;
 
 	if ($d->{type} eq 'OVSBond') {
 
@@ -1079,10 +1075,10 @@ sub __interface_to_string {
 		&$set_ovs_option($d, lacp => undef);
 		&$set_ovs_option($d, bond_mode => $d->{bond_mode});
 	    }
-	    $done->{bond_mode} = 1;
+	    $once->{bond_mode} = 1;
 
 	    $raw .= "\tovs_bonds $d->{ovs_bonds}\n" if $d->{ovs_bonds};
-	    $done->{ovs_bonds} = 1;
+	    $once->{ovs_bonds} = 1;
 	}
 
 	if ($d->{ovs_bridge}) {
@@ -1090,20 +1086,28 @@ sub __interface_to_string {
 	}
 
 	$raw .= "\tovs_type $d->{type}\n";
-	$done->{ovs_type} = 1;
+	$once->{ovs_type} = 1;
 
 	if ($d->{ovs_bridge}) {
 	    $raw .= "\tovs_bridge $d->{ovs_bridge}\n";
-	    $done->{ovs_bridge} = 1;
+	    $once->{ovs_bridge} = 1;
 	}
 	# fixme: use Data::Dumper; print Dumper($d);
     }
 
+    my $done = { type => 1, priority => 1, method => 1, active => 1, exists => 1,
+		 comments => 1, autostart => 1, options => 1,
+		 address => 1, netmask => 1, gateway => 1, broadcast => 1,
+		 method6=> 1, families => 1,
+		 address6 => 1, netmask6 => 1, gateway6 => 1, broadcast6 => 1,
+		 %$once };
+
     # print other settings
     foreach my $k (keys %$d) {
 	next if $done->{$k};
 	next if !$d->{$k};
 	$raw .= "\t$k $d->{$k}\n";
+	$once->{$k} = 1;
     }
 
     foreach my $option (@{$d->{"options$suffix"}}) {
@@ -1261,7 +1265,8 @@ sub write_etc_network_interfaces {
 
 	$printed->{$iface} = 1;
 	$raw .= "auto $iface\n" if $d->{autostart};
-	$raw .= __interface_to_string($iface, $d, $_) for @{$d->{families}};
+	my $once = {}; # some options should be printed only once
+	$raw .= __interface_to_string($iface, $d, $_, $once) foreach @{$d->{families}};
     }
 
     $raw .= $_->[1] . "\n" foreach @options;
-- 
2.1.4