[pve-devel] [PATCH common] Fix #132: hold a lock while setting up vlan bridges

[Wolfgang Bumiller]

applied

On Mon, Jun 13, 2016 at 02:36:09PM +0200, Wolfgang Bumiller wrote:
> It's possible for two simultaneous VM starts to try to
> create vlan bridges on non-vlan-aware bridges
> simultaneously, which causes one of them to fail with the
> error "can't add bridge ...".
> ---
> As far as I can tell this is the only place reachable via the
> pve-bridge script where this can happen.
> The rest of the code is commands on the bridge port itself or on the
> VM-owned firewall bridges.
> 
>  src/PVE/Network.pm | 38 +++++++++++++++++++++++---------------
>  1 file changed, 23 insertions(+), 15 deletions(-)
> 
> diff --git a/src/PVE/Network.pm b/src/PVE/Network.pm
> index 63476d3..93afcf8 100644
> --- a/src/PVE/Network.pm
> +++ b/src/PVE/Network.pm
> @@ -2,7 +2,7 @@ package PVE::Network;
>  
>  use strict;
>  use warnings;
> -use PVE::Tools qw(run_command);
> +use PVE::Tools qw(run_command lock_file);
>  use PVE::ProcFSTools;
>  use PVE::INotify;
>  use File::Basename;
> @@ -452,23 +452,24 @@ sub activate_bridge_vlan {
>  
>      die "no physical interface on bridge '$bridge'\n" if scalar(@ifaces) == 0;
>  
> -    # add bridgevlan if it doesn't already exist
> -    if (! -d "/sys/class/net/$bridgevlan") {
> -        system("/sbin/brctl addbr $bridgevlan") == 0 ||
> -            die "can't add bridge $bridgevlan\n";
> -    }
> +    lock_network(sub {
> +	# add bridgevlan if it doesn't already exist
> +	if (! -d "/sys/class/net/$bridgevlan") {
> +	    system("/sbin/brctl addbr $bridgevlan") == 0 ||
> +		die "can't add bridge $bridgevlan\n";
> +	}
>  
> -    # for each physical interface (eth or bridge) bind them to bridge vlan
> -    foreach my $iface (@ifaces) {
> -        activate_bridge_vlan_slave($bridgevlan, $iface, $tag);
> -    }
> +	# for each physical interface (eth or bridge) bind them to bridge vlan
> +	foreach my $iface (@ifaces) {
> +	    activate_bridge_vlan_slave($bridgevlan, $iface, $tag);
> +	}
>  
> -    #fixme: set other bridge flags
> +	#fixme: set other bridge flags
>  
> -    # be sure to have the bridge up
> -    system("/sbin/ip link set $bridgevlan up") == 0 ||
> -        die "can't up bridge $bridgevlan\n";
> -   
> +	# be sure to have the bridge up
> +	system("/sbin/ip link set $bridgevlan up") == 0 ||
> +	    die "can't up bridge $bridgevlan\n";
> +    });
>      return $bridgevlan;
>  }
>  
> @@ -531,4 +532,11 @@ sub is_ip_in_cidr {
>      return $cidr_obj->overlaps($ip_obj) == $Net::IP::IP_B_IN_A_OVERLAP;
>  }
>  
> +sub lock_network {
> +    my ($code, @param) = @_;
> +    my $res = lock_file('/var/lock/pve-network.lck', 10, $code, @param);
> +    die $@ if $@;
> +    return $res;
> +}
> +
>  1;
> -- 
> 2.1.4
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>