diff --git a/lib/vagrant-libvirt/action/create_network_interfaces.rb b/lib/vagrant-libvirt/action/create_network_interfaces.rb index d353363..3b98b24 100644 --- a/lib/vagrant-libvirt/action/create_network_interfaces.rb +++ b/lib/vagrant-libvirt/action/create_network_interfaces.rb @@ -61,7 +61,7 @@ module VagrantPlugins # We have slot for interface, fill it with interface configuration. adapters[free_slot] = options adapters[free_slot][:network_name] = interface_network( - env[:machine].provider.driver.connection.client, adapters[free_slot] + env[:machine].provider.driver, adapters[free_slot] ) end @@ -282,7 +282,7 @@ module VagrantPlugins end # Return network name according to interface options. - def interface_network(libvirt_client, options) + def interface_network(driver, options) # no need to get interface network for tcp tunnel config return 'tunnel_interface' if options.fetch(:tunnel_type, nil) @@ -292,7 +292,7 @@ module VagrantPlugins end # Get list of all (active and inactive) Libvirt networks. - available_networks = libvirt_networks(libvirt_client) + available_networks = libvirt_networks(driver) return 'public' if options[:iface_type] == :public_network diff --git a/lib/vagrant-libvirt/action/create_networks.rb b/lib/vagrant-libvirt/action/create_networks.rb index ee2b451..775aba5 100644 --- a/lib/vagrant-libvirt/action/create_networks.rb +++ b/lib/vagrant-libvirt/action/create_networks.rb @@ -35,9 +35,7 @@ module VagrantPlugins # for VMs using sessions. It is likely that this should be done # to determine the correct virtual device for the management # network for sessions instead of assuming the default of virbr0. - @available_networks = libvirt_networks( - env[:machine].provider.driver.system_connection - ) + @available_networks = libvirt_networks(env[:machine].provider.driver) @app.call(env) return @@ -61,9 +59,7 @@ module VagrantPlugins # Get a list of all (active and inactive) Libvirt networks. This # list is used throughout this class and should be easier to # process than Libvirt API calls. - @available_networks = libvirt_networks( - env[:machine].provider.driver.connection.client - ) + @available_networks = libvirt_networks(env[:machine].provider.driver) current_network = @available_networks.detect { |network| network[:name] == @options[:network_name] } diff --git a/lib/vagrant-libvirt/driver.rb b/lib/vagrant-libvirt/driver.rb index 18b928f..5b834d4 100644 --- a/lib/vagrant-libvirt/driver.rb +++ b/lib/vagrant-libvirt/driver.rb @@ -197,6 +197,19 @@ module VagrantPlugins domain end + def list_all_networks + system_connection.list_all_networks.select do |net| + begin + net.bridge_name + rescue Libvirt::Error + # there does not appear to be a mechanism to determine the type of network, only by + # querying the attribute and catching the error is it possible to ignore unsupported. + @logger.debug "Ignoring #{net.name} as it does not support retrieval of bridge_name attribute" + next + end + end + end + def host_devices @host_devices ||= begin cmd = [] @@ -216,7 +229,7 @@ module VagrantPlugins ( info.map { |iface| iface['ifname'] } + connection.client.list_all_interfaces.map { |iface| iface.name } + - connection.client.list_all_networks.map { |net| net.bridge_name } + list_all_networks.map { |net| net.bridge_name } ).uniq.reject(&:empty?) end end @@ -234,7 +247,7 @@ module VagrantPlugins def get_ipaddress_from_system(mac) ip_address = nil - system_connection.list_all_networks.each do |net| + list_all_networks.each do |net| leases = net.dhcp_leases(mac, 0) # Assume the lease expiring last is the current IP address ip_address = leases.max_by { |lse| lse['expirytime'] }['ipaddr'] unless leases.empty? diff --git a/lib/vagrant-libvirt/util/network_util.rb b/lib/vagrant-libvirt/util/network_util.rb index e1e85e0..2e7cc60 100644 --- a/lib/vagrant-libvirt/util/network_util.rb +++ b/lib/vagrant-libvirt/util/network_util.rb @@ -149,19 +149,11 @@ module VagrantPlugins # Return a list of all (active and inactive) Libvirt networks as a list # of hashes with their name, network address and status (active or not) - def libvirt_networks(libvirt_client) + def libvirt_networks(driver) libvirt_networks = [] # Iterate over all (active and inactive) networks. - libvirt_client.list_all_networks.each do |libvirt_network| - begin - bridge_name = libvirt_network.bridge_name - rescue Libvirt::Error - # there does not appear to be a mechanism to determine the type of network, only by - # querying the attribute and catching the error is it possible to ignore unsupported. - @logger.debug "Ignoring #{libvirt_network.name} as it does not support retrieval of bridge_name attribute" - next - end + driver.list_all_networks.each do |libvirt_network| # Parse ip address and netmask from the network xml description. xml = Nokogiri::XML(libvirt_network.xml_desc) @@ -189,7 +181,7 @@ module VagrantPlugins netmask: netmask, network_address: network_address, dhcp_enabled: dhcp_enabled, - bridge_name: bridge_name, + bridge_name: libvirt_network.bridge_name, domain_name: domain_name, created: true, active: libvirt_network.active?, diff --git a/spec/unit/driver_spec.rb b/spec/unit/driver_spec.rb index 6b08635..8250115 100644 --- a/spec/unit/driver_spec.rb +++ b/spec/unit/driver_spec.rb @@ -187,7 +187,7 @@ describe VagrantPlugins::ProviderLibvirt::Driver do context 'when qemu_use_session is enabled' do let(:system_connection) { double("system connection") } - let(:networks) { [instance_double(::Fog::Libvirt::Compute::Real)] } + let(:networks) { [instance_double(::Libvirt::Network)] } let(:dhcp_leases) { { "iface" =>"virbr0", @@ -207,8 +207,7 @@ describe VagrantPlugins::ProviderLibvirt::Driver do it 'should retrieve the address via the system dhcp-leases API' do expect(domain).to receive(:mac).and_return("52:54:00:8b:dc:5f") - expect(subject).to receive(:system_connection).and_return(system_connection) - expect(system_connection).to receive(:list_all_networks).and_return(networks) + expect(subject).to receive(:list_all_networks).and_return(networks) expect(networks[0]).to receive(:dhcp_leases).and_return([dhcp_leases]) expect(subject.get_ipaddress).to eq("192.168.122.43") @@ -229,6 +228,43 @@ describe VagrantPlugins::ProviderLibvirt::Driver do end end + + describe '#list_all_networks' do + let(:vagrantfile_providerconfig) do + <<-EOF + libvirt.uri = "qemu:///system" + EOF + end + + let(:libvirt_networks) { [ + instance_double(::Libvirt::Network), + instance_double(::Libvirt::Network), + instance_double(::Libvirt::Network), + ] } + + before do + allow(subject).to receive(:system_connection).and_return(libvirt_client) + expect(libvirt_client).to receive(:list_all_networks).and_return(libvirt_networks) + end + + it 'should list networks' do + expect(libvirt_networks[0]).to receive(:bridge_name).and_return('') + expect(libvirt_networks[1]).to receive(:bridge_name).and_return('virbr0') + expect(libvirt_networks[2]).to receive(:bridge_name).and_return('virbr1') + + expect(subject.list_all_networks).to eq(libvirt_networks) + end + + it 'should skip networks missing bridge_name' do + expect(libvirt_networks[0]).to receive(:bridge_name).and_return('') + expect(libvirt_networks[1]).to receive(:bridge_name).and_raise(Libvirt::Error) + expect(libvirt_networks[1]).to receive(:name).and_return('bad_network') + expect(libvirt_networks[2]).to receive(:bridge_name).and_return('virbr1') + + expect(subject.list_all_networks).to eq([libvirt_networks[0], libvirt_networks[2]]) + end + end + describe '#host_devices' do let(:vagrantfile_providerconfig) do <<-EOF @@ -265,7 +301,7 @@ describe VagrantPlugins::ProviderLibvirt::Driver do end.and_return(Vagrant::Util::Subprocess::Result.new(exit_code=0, stdout=ip_link_show, stderr='')) expect(libvirt_client).to receive(:list_all_interfaces).and_return(libvirt_interfaces) - expect(libvirt_client).to receive(:list_all_networks).and_return(libvirt_networks) + expect(subject).to receive(:list_all_networks).and_return(libvirt_networks) expect(libvirt_interfaces[0]).to receive(:name).and_return('eth0') expect(libvirt_interfaces[1]).to receive(:name).and_return('virbr0') expect(libvirt_networks[0]).to receive(:bridge_name).and_return('') diff --git a/spec/unit/util/network_util_spec.rb b/spec/unit/util/network_util_spec.rb index 0f7e4d3..c63e0b0 100644 --- a/spec/unit/util/network_util_spec.rb +++ b/spec/unit/util/network_util_spec.rb @@ -45,23 +45,12 @@ describe 'VagrantPlugins::ProviderLibvirt::Util::NetworkUtil' do describe '#libvirt_networks' do let(:default_network) { create_libvirt_network('default') } let(:additional_network) { create_libvirt_network('vagrant-libvirt') } - let(:hostdev_network) { create_libvirt_network('hostdev', {:active? => false}) } it 'should retrieve the list of networks' do expect(logger).to_not receive(:debug) - expect(libvirt_client).to receive(:list_all_networks).and_return([default_network, additional_network]) + expect(driver).to receive(:list_all_networks).and_return([default_network, additional_network]) - expect(subject.libvirt_networks(libvirt_client)).to match_array([ - hash_including(:name => 'default'), - hash_including(:name => 'vagrant-libvirt'), - ]) - end - - it 'should handle networks without bridge names' do - expect(logger).to receive(:debug).with(/Ignoring hostdev as it does not/) - expect(libvirt_client).to receive(:list_all_networks).and_return([default_network, hostdev_network, additional_network]) - - expect(subject.libvirt_networks(libvirt_client)).to match_array([ + expect(subject.libvirt_networks(driver)).to match_array([ hash_including(:name => 'default'), hash_including(:name => 'vagrant-libvirt'), ])