Ignore networks that cannot be used (#1628)

If the network does not have a bridge name, ignore it and move onto the
next one. This allows for hostdev networks to exist without breaking.

Includes some rudimentary testing to exercise the lookup code along with
a small bit of refactoring based on the realisation that there is no
need to lookup the network information twice as it is available if the
list_all_networks API is used.

Fixes: #599

lib/vagrant-libvirt/util/network_util.rb

@@ -152,14 +152,16 @@ module VagrantPlugins
         def libvirt_networks(libvirt_client)
           libvirt_networks = []
 
-          active = libvirt_client.list_networks
-          inactive = libvirt_client.list_defined_networks
-
           # Iterate over all (active and inactive) networks.
-          active.concat(inactive).each do |network_name|
-            libvirt_network = libvirt_client.lookup_network_by_name(
-              network_name
-            )
+          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
 
             # Parse ip address and netmask from the network xml description.
             xml = Nokogiri::XML(libvirt_network.xml_desc)
@@ -182,12 +184,12 @@ module VagrantPlugins
             network_address = (network_address(ip, netmask) if ip && netmask)
 
             libvirt_networks << {
-              name:             network_name,
+              name:             libvirt_network.name,
               ip_address:       ip,
               netmask:          netmask,
               network_address:  network_address,
               dhcp_enabled:     dhcp_enabled,
-              bridge_name:      libvirt_network.bridge_name,
+              bridge_name:      bridge_name,
               domain_name:      domain_name,
               created:          true,
               active:           libvirt_network.active?,

spec/unit/util/network_util_spec.rb

@@ -0,0 +1,70 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+require 'vagrant-libvirt/util/network_util'
+
+describe 'VagrantPlugins::ProviderLibvirt::Util::NetworkUtil' do
+  include_context 'libvirt'
+
+  subject do
+    Class.new {
+      include VagrantPlugins::ProviderLibvirt::Util::NetworkUtil
+
+      def initialize
+        @logger = Log4r::Logger.new('test-logger')
+      end
+    }.new
+  end
+
+  def create_libvirt_network(name, attrs={})
+    default_attrs = {
+      :active? => true,
+      :autostart? => true,
+    }
+    network_xml = File.read(File.join(File.dirname(__FILE__), File.basename(__FILE__, '.rb'), name + '.xml'))
+    double = instance_double(::Libvirt::Network)
+    allow(double).to receive(:xml_desc).and_return(network_xml)
+    allow(double).to receive(:name).and_return(name)
+
+    xml = REXML::Document.new(network_xml)
+    bridge = REXML::XPath.first(xml, '/network/bridge')
+    default_attrs[:bridge_name] = !bridge.nil? ? bridge.attributes['name'] : Libvirt::Error.new("network #{name} does not have attribute bridge_name")
+
+    default_attrs.merge(attrs).each do |aname, avalue|
+      if avalue.is_a?(Exception)
+        allow(double).to receive(aname).and_raise(avalue)
+      else
+        allow(double).to receive(aname).and_return(avalue)
+      end
+    end
+
+    double
+  end
+
+  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(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([
+        hash_including(:name => 'default'),
+        hash_including(:name => 'vagrant-libvirt'),
+      ])
+    end
+  end
+end

spec/unit/util/network_util_spec/default.xml

@@ -0,0 +1,16 @@
+<network>
+  <name>default</name>
+  <uuid>39e02111-c66c-4653-86a9-8d20673a4eb1</uuid>
+  <forward mode='nat'>
+    <nat>
+      <port start='1024' end='65535'/>
+    </nat>
+  </forward>
+  <bridge name='virbr0' stp='on' delay='0'/>
+  <mac address='74-c3-fd-27-a8-c9'/>
+  <ip address='192.168.122.1' netmask='255.255.255.0'>
+    <dhcp>
+      <range start='192.168.122.2' end='192.168.122.254'/>
+    </dhcp>
+  </ip>
+</network>

spec/unit/util/network_util_spec/hostdev.xml

@@ -0,0 +1,6 @@
+<network>
+  <name>passthrough</name>
+  <forward mode='hostdev' managed='yes'>
+    <pf dev='eth3'/>
+  </forward>
+</network>

spec/unit/util/network_util_spec/vagrant-libvirt.xml

@@ -0,0 +1,16 @@
+<network connections='1' ipv6='yes'>
+  <name>vagrant-libvirt</name>
+  <uuid>7347e8a9-bd25-409c-b4c3-ebf8744322bc</uuid>
+  <forward mode='nat'>
+    <nat>
+      <port start='1024' end='65535'/>
+    </nat>
+  </forward>
+  <bridge name='virbr1' stp='on' delay='0'/>
+  <mac address='52:54:00:d1:56:08'/>
+  <ip address='192.168.121.1' netmask='255.255.255.0'>
+    <dhcp>
+      <range start='192.168.121.1' end='192.168.121.254'/>
+    </dhcp>
+  </ip>
+</network>