On Mon, Jun 25, 2018 at 05:41:35AM +0200, Alexandre Derumier wrote:
> ---
> src/PVE/INotify.pm | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 51 insertions(+), 2 deletions(-)
>
> diff --git a/src/PVE/INotify.pm b/src/PVE/INotify.pm
> index 2c1af3c..2401ec0 100644
> --- a/src/PVE/INotify.pm
> +++ b/src/PVE/INotify.pm
> @@ -898,7 +898,9 @@ sub __read_etc_network_interfaces {
> } else {
> $d->{$id} = 'off';
> }
> - } elsif ($id eq 'bridge_fd' || $id eq 'bridge_vids') {
> + } elsif ($id eq 'bridge_fd' || $id eq 'bridge_vids' || $id
> eq 'bridge-access' ||
> + $id eq 'bridge-learning' || $id eq
> 'bridge-arp-nd-suppress' ||
> + $id eq 'bridge-unicast-flood' || $id eq
> 'bridge-multicast-flood') {
> $d->{$id} = $value;
At some point we need to clean this up and just use a hash declaring
which ids are copied as-is via `$d->{$id} = $value;`
(This is mostly a reminder for me, but if you feel up to it... ;-) )
> } elsif ($id eq 'bridge_vlan_aware') {
> $d->{$id} = 1;
> @@ -1150,7 +1152,22 @@ sub __interface_to_string {
> }
> $done->{'vxlan-remoteip'} = 1;
> }
> -
> + if ($d->{'bridge-learning'}) {
Particularly together with the other patch this pattern is really worth
iterating over via a list now ;-)
> + $raw .= "\tbridge-learning $d->{'bridge-learning'}\n";
> + $done->{'bridge-learning'} = 1;
> + }
> + if ($d->{'bridge-arp-nd-suppress'}) {
> + $raw .= "\tbridge-arp-nd-suppress $d->{'bridge-arp-nd-suppress'}\n";
> + $done->{'bridge-arp-nd-suppress'} = 1;
> + }
> + if ($d->{'bridge-unicast-flood'}) {
> + $raw .= "\tbridge-unicast-flood $d->{'bridge-unicast-flood'}\n";
> + $done->{'bridge-unicast-flood'} = 1;
> + }
> + if ($d->{'bridge-multicast-flood'}) {
> + $raw .= "\tbridge-multicast-flood $d->{'bridge-multicast-flood'}\n";
> + $done->{'bridge-multicast-flood'} = 1;
> + }
> } elsif ($d->{type} eq 'OVSBridge') {
>
> $raw .= "\tovs_type $d->{type}\n";
> @@ -1194,6 +1211,13 @@ sub __interface_to_string {
> $raw .= "\tovs_type $d->{type}\n";
> $done->{ovs_type} = 1;
>
> + if ($d->{type} eq 'eth' || $d->{type} eq 'bond' || $d->{type} eq
> 'vxlan') {
Is this specifically limited to these types of interfaces? Since below
we check whether the interface is part of a bridge I wonder if we should
just skip the type condition. Basically any port which can't be on a
bridge should be caught elsewhere already anyway.
> + if ($d->{'bridge-access'}) {
> + $raw .= "\tbridge-access $d->{'bridge-access'}\n";
> + $done->{'bridge-access'} = 1;
> + }
> + }
> +
> if ($d->{ovs_bridge}) {
>
> if ($ifupdown2) {
> @@ -1344,6 +1368,31 @@ sub __write_etc_network_interfaces {
> }
> }
>
> + # check bridgeport option
> + my $bridgeports = {};
> + my $bridges = {};
> + foreach my $iface (keys %$ifaces) {
> + my $d = $ifaces->{$iface};
> + if ($d->{type} eq 'bridge') {
> + foreach my $p (split (/\s+/, $d->{bridge_ports})) {
> + $bridgeports->{$p} = $iface;
> + }
> + $bridges->{$iface} = $d;
> + }
> + }
> +
> + foreach my $iface (keys %$ifaces) {
> + my $d = $ifaces->{$iface};
> + if ($d->{'bridge-learning'} || $d->{'bridge-arp-nd-suppress'} ||
> $d->{'bridge-unicast-flood'} ||
> + $d->{'bridge-multicast-flood'} || $d->{'bridge-access'}) {
> + die "iface $iface : bridgeports options can be used only if
> interface is in a bridge" if !$bridgeports->{$iface};
double whitespace, 'bridgeports' might be misleading, this is about
bridge-related options. Perhaps we could gather them in a list and print
them with the error (would look nicer in code than the big `or` chain
anyway.
And a missing \n at the end of the error message.
> + }
> +
> + if ($d->{'bridge-access'} &&
> !$bridges->{$bridgeports->{$iface}}->{bridge_vlan_aware}) {
> + die "iface $iface : bridge-access option can be only used if
> interface is in a vlan aware bridge";
missing \n at the end of the error message.
> + }
> + }
> +
> my $raw = <<'NETWORKDOC';
> # network interface settings; autogenerated
> # Please do NOT modify this file directly, unless you know what
> --
> 2.11.0
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
