From 41f16c9b591bf6f05d54c312305df07fee5b064c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Andr=C3=A9?= Date: Wed, 23 Sep 2015 22:24:44 +0900 Subject: [PATCH 1/2] Handle private networks with type DHCP Closes #427 --- .../action/create_network_interfaces.rb | 4 +- lib/vagrant-libvirt/action/create_networks.rb | 52 +++++++++++++++++-- lib/vagrant-libvirt/util/network_util.rb | 5 ++ 3 files changed, 54 insertions(+), 7 deletions(-) diff --git a/lib/vagrant-libvirt/action/create_network_interfaces.rb b/lib/vagrant-libvirt/action/create_network_interfaces.rb index 49e44ee..866bf54 100644 --- a/lib/vagrant-libvirt/action/create_network_interfaces.rb +++ b/lib/vagrant-libvirt/action/create_network_interfaces.rb @@ -119,7 +119,7 @@ module VagrantPlugins end # Re-read the network configuration and grab the MAC address - if !@mac + unless @mac xml = Nokogiri::XML(domain.xml_desc) if iface_configuration[:iface_type] == :public_network if @type == 'direct' @@ -184,7 +184,7 @@ module VagrantPlugins def find_empty(array, start=0, stop=@nic_adapter_count) (start..stop).each do |i| - return i if !array[i] + return i unless array[i] end return nil end diff --git a/lib/vagrant-libvirt/action/create_networks.rb b/lib/vagrant-libvirt/action/create_networks.rb index 60a1d65..965c2d7 100644 --- a/lib/vagrant-libvirt/action/create_networks.rb +++ b/lib/vagrant-libvirt/action/create_networks.rb @@ -35,7 +35,7 @@ module VagrantPlugins # available, create it if possible. Otherwise raise an error. configured_networks(env, @logger).each do |options| # Only need to create private networks - next if options[:iface_type] != :private_network or + next if options[:iface_type] != :private_network || options.fetch(:tunnel_type, nil) @logger.debug "Searching for network with options #{options}" @@ -63,12 +63,16 @@ module VagrantPlugins if @options[:ip] handle_ip_option(env) + elsif @options[:type] == :dhcp + handle_dhcp_private_network(env) elsif @options[:network_name] handle_network_name_option(env) + else + raise Errors::CreateNetworkError, error_message: @options end autostart_network if @interface_network[:autostart] - activate_network if !@interface_network[:active] + activate_network unless @interface_network[:active] end end @@ -126,7 +130,7 @@ module VagrantPlugins # Handle only situations, when ip is specified. Variables @options and # @available_networks should be filled before calling this function. def handle_ip_option(env) - return if !@options[:ip] + return unless @options[:ip] net_address = nil if @options[:forward_mode] != 'veryisolated' net_address = network_address(@options[:ip], @options[:netmask]) @@ -185,7 +189,7 @@ module VagrantPlugins end # Do we need to create new network? - if !@interface_network[:created] + unless @interface_network[:created] # TODO: stop after some loops. Don't create infinite loops. @@ -241,7 +245,7 @@ module VagrantPlugins end # Do we need to create new network? - if !@interface_network[:created] + unless @interface_network[:created] @interface_network[:name] = @options[:network_name] # Generate a unique name for network bridge. @@ -262,6 +266,44 @@ module VagrantPlugins end end + def handle_dhcp_private_network(env) + network = lookup_network_by_ip('172.28.128.0') + @interface_network = network if network + + # Do we need to create new network? + unless @interface_network[:created] + @interface_network[:name] = 'vagrant-private-dhcp' + + net_address = '172.28.128.0' + @interface_network[:network_address] = net_address + + # Set IP address of network (actually bridge). It will be used as + # gateway address for machines connected to this network. + net = IPAddr.new(net_address) + + # Default to first address (after network name) + @interface_network[:ip_address] = @options[:host_ip].nil? ? \ + net.to_range.begin.succ : \ + IPAddr.new(@options[:host_ip]) + + # Generate a unique name for network bridge. + count = 0 + while @interface_network[:bridge_name].nil? + @logger.debug "generating name for bridge" + bridge_name = 'virbr' + bridge_name << count.to_s + count += 1 + + next if lookup_bridge_by_name(bridge_name) + + @interface_network[:bridge_name] = bridge_name + end + + # Create a private network. + create_private_network(env) + end + end + def create_private_network(env) @network_name = @interface_network[:name] @network_bridge_name = @interface_network[:bridge_name] diff --git a/lib/vagrant-libvirt/util/network_util.rb b/lib/vagrant-libvirt/util/network_util.rb index 3e0bd2b..44c9ab2 100644 --- a/lib/vagrant-libvirt/util/network_util.rb +++ b/lib/vagrant-libvirt/util/network_util.rb @@ -59,6 +59,11 @@ module VagrantPlugins dhcp_enabled: true, forward_mode: 'nat', }.merge(options) + + if options[:type] == :dhcp && !options[:ip] + options[:network_name] = "vagrant-private-dhcp" + end + # add to list of networks to check networks.push(options) end From 3f29a98a4ed8a66265a082f20a8d6f2685ffe817 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Andr=C3=A9?= Date: Thu, 24 Sep 2015 10:32:22 +0900 Subject: [PATCH 2/2] Refactoring Reduce code duplication and make things more clear --- lib/vagrant-libvirt/action/create_networks.rb | 95 ++++++------------- 1 file changed, 28 insertions(+), 67 deletions(-) diff --git a/lib/vagrant-libvirt/action/create_networks.rb b/lib/vagrant-libvirt/action/create_networks.rb index 965c2d7..ae18910 100644 --- a/lib/vagrant-libvirt/action/create_networks.rb +++ b/lib/vagrant-libvirt/action/create_networks.rb @@ -83,34 +83,19 @@ module VagrantPlugins def lookup_network_by_ip(ip) @logger.debug "looking up network with ip == #{ip}" - @available_networks.each do |network| - if network[:network_address] == ip - @logger.debug "found existing network by ip: #{network}" - return network - end - end - nil + @available_networks.find { |network| network[:network_address] == ip } end # Return hash of network for specified name, or nil if not found. def lookup_network_by_name(network_name) @logger.debug "looking up network named #{network_name}" - @available_networks.each do |network| - if network[:name] == network_name - @logger.debug "found existing network by name: #{network}" - return network - end - end - nil + @available_networks.find { |network| network[:name] == network_name } end # Return hash of network for specified bridge, or nil if not found. def lookup_bridge_by_name(bridge_name) @logger.debug "looking up bridge named #{bridge_name}" - @available_networks.each do |network| - return network if network[:bridge_name] == bridge_name - end - nil + @available_networks.find { |network| network[:bridge_name] == bridge_name } end # Throw an error if dhcp setting for an existing network does not @@ -132,16 +117,12 @@ module VagrantPlugins def handle_ip_option(env) return unless @options[:ip] net_address = nil - if @options[:forward_mode] != 'veryisolated' + unless @options[:forward_mode] == 'veryisolated' net_address = network_address(@options[:ip], @options[:netmask]) + # Set IP address of network (actually bridge). It will be used as # gateway address for machines connected to this network. - net = IPAddr.new(net_address) - - # Default to first address (after network name) - @interface_network[:ip_address] = @options[:host_ip].nil? ? \ - net.to_range.begin.succ : \ - IPAddr.new(@options[:host_ip]) + @interface_network[:ip_address] = get_host_ip_addr(net_address) end @interface_network[:network_address] = net_address @@ -210,17 +191,7 @@ module VagrantPlugins end # Generate a unique name for network bridge. - count = 0 - while @interface_network[:bridge_name].nil? - @logger.debug "generating name for bridge" - bridge_name = 'virbr' - bridge_name << count.to_s - count += 1 - - next if lookup_bridge_by_name(bridge_name) - - @interface_network[:bridge_name] = bridge_name - end + @interface_network[:bridge_name] = generate_bridge_name # Create a private network. create_private_network(env) @@ -249,17 +220,7 @@ module VagrantPlugins @interface_network[:name] = @options[:network_name] # Generate a unique name for network bridge. - count = 0 - while @interface_network[:bridge_name].nil? - @logger.debug "generating name for bridge" - bridge_name = 'virbr' - bridge_name << count.to_s - count += 1 - - next if lookup_bridge_by_name(bridge_name) - - @interface_network[:bridge_name] = bridge_name - end + @interface_network[:bridge_name] = generate_bridge_name # Create a private network. create_private_network(env) @@ -267,43 +228,43 @@ module VagrantPlugins end def handle_dhcp_private_network(env) - network = lookup_network_by_ip('172.28.128.0') + net_address = '172.28.128.0' + network = lookup_network_by_ip(net_address) @interface_network = network if network # Do we need to create new network? unless @interface_network[:created] @interface_network[:name] = 'vagrant-private-dhcp' - - net_address = '172.28.128.0' @interface_network[:network_address] = net_address # Set IP address of network (actually bridge). It will be used as # gateway address for machines connected to this network. - net = IPAddr.new(net_address) - - # Default to first address (after network name) - @interface_network[:ip_address] = @options[:host_ip].nil? ? \ - net.to_range.begin.succ : \ - IPAddr.new(@options[:host_ip]) + @interface_network[:ip_address] = get_host_ip_addr(net_address) # Generate a unique name for network bridge. - count = 0 - while @interface_network[:bridge_name].nil? - @logger.debug "generating name for bridge" - bridge_name = 'virbr' - bridge_name << count.to_s - count += 1 - - next if lookup_bridge_by_name(bridge_name) - - @interface_network[:bridge_name] = bridge_name - end + @interface_network[:bridge_name] = generate_bridge_name # Create a private network. create_private_network(env) end end + # Return provided address or first address of network otherwise + def get_host_ip_addr(network) + @options[:host_ip] ? IPAddr.new(@options[:host_ip]) : IPAddr.new(network).succ + end + + # Return the first available virbr interface name + def generate_bridge_name + @logger.debug "generating name for bridge" + count = 0 + while lookup_bridge_by_name(bridge_name = "virbr#{count}") + count += 1 + end + @logger.debug "found available bridge name #{bridge_name}" + bridge_name + end + def create_private_network(env) @network_name = @interface_network[:name] @network_bridge_name = @interface_network[:bridge_name]