Handle different attribute and element ordering (#1592)

Normalise the XML to ensure the attributes for both documents have the
same ordering to prevent excessive noise when differences are detected.
Additionally sort various elements based on attributes that make
ordering irrelevant to allow for simpler comparison using xmlsimple.

Closes: #1583

lib/vagrant-libvirt/action/start_domain.rb

@@ -429,9 +429,20 @@ module VagrantPlugins
               raise Errors::UpdateServerError, error_message: e.message
             end
 
+            # this normalises the attribute order to be the same as what was sent in the above
+            # request to update the domain XML. Without this, if the XML documents are not
+            # equivalent, many more differences will be reported than there actually are.
+            applied_xml = String.new
+            REXML::Document.new(libvirt_domain.xml_desc(1)).write(applied_xml)
+
             # need to check whether the updated XML contains all the changes requested
             proposed = VagrantPlugins::ProviderLibvirt::Util::Xml.new(new_xml)
-            applied = VagrantPlugins::ProviderLibvirt::Util::Xml.new(libvirt_domain.xml_desc(1))
+            applied = VagrantPlugins::ProviderLibvirt::Util::Xml.new(applied_xml)
+
+            # perform some sorting to allow comparison otherwise order of devices differing
+            # even if they are equivalent will be reported as being different.
+            proposed.xml['devices'][0].each { |_, v| next unless v.is_a?(Array); v.sort_by! { |e| [e['type'], e['index']]} }
+            applied.xml['devices'][0].each { |_, v| next unless v.is_a?(Array); v.sort_by! { |e| [e['type'], e['index']]} }
 
             if proposed != applied
               require 'diffy'

spec/spec_helper.rb

@@ -63,6 +63,10 @@ RSpec.configure do |config|
 
   # don't run acceptance tests by default
   config.filter_run_excluding :acceptance => true
+
+  config.expect_with :rspec do |c|
+    c.max_formatted_output_length = 2000 if c.respond_to?("max_formatted_output_length=")
+  end
 end
 
 require 'vagrant-spec/unit'

spec/unit/action/start_domain_spec.rb

@@ -74,6 +74,26 @@ describe VagrantPlugins::ProviderLibvirt::Action::StartDomain do
       end
     end
 
+    context 'when xml elements and attributes reordered' do
+      let(:test_file) { 'existing.xml' }
+      let(:updated_test_file) { 'existing_reordered.xml' }
+      let(:vagrantfile_providerconfig) do
+        <<-EOF
+        libvirt.cpu_mode = "host-passthrough"
+        EOF
+      end
+
+      it 'should correctly detect the domain was updated' do
+        expect(ui).to_not receive(:warn)
+        expect(libvirt_domain).to receive(:autostart=)
+        expect(connection).to receive(:define_domain).and_return(libvirt_domain)
+        expect(libvirt_domain).to receive(:xml_desc).and_return(domain_xml, updated_domain_xml)
+        expect(domain).to receive(:start)
+
+        expect(subject.call(env)).to be_nil
+      end
+    end
+
     context 'when xml not applied' do
       let(:test_file) { 'default_with_different_formatting.xml' }
       let(:updated_domain_xml) {

spec/unit/action/start_domain_spec/existing_reordered.xml

@@ -0,0 +1,62 @@
+<domain type='qemu'>
+  <name>vagrant-libvirt_default</name>
+  <uuid>881a931b-0110-4d10-81aa-47a1a19f5726</uuid>
+  <description>Source: /home/test/vagrant-libvirt/Vagrantfile</description>
+  <memory unit='KiB'>2097152</memory>
+  <currentMemory unit='KiB'>2097152</currentMemory>
+  <vcpu placement='static'>2</vcpu>
+  <os>
+    <type arch='x86_64' machine='pc-i440fx-6.0'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <features>
+    <acpi/>
+    <apic/>
+    <pae/>
+  </features>
+  <cpu check='partial' mode='host-passthrough'/>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-x86_64</emulator>
+    <disk device='disk' type='file'>
+      <driver name='qemu' type='qcow2'/>
+      <source file='/var/lib/libvirt/images/vagrant-libvirt_default.img'/>
+      <target bus='virtio' dev='vda'/>
+      <address bus='0x00' domain='0x0000' function='0x0' slot='0x03' type='pci'/>
+    </disk>
+    <controller index='0' model='pci-root' type='pci'/>
+    <controller index='0' model='piix3-uhci' type='usb'>
+      <address bus='0x00' domain='0x0000' function='0x2' slot='0x01' type='pci'/>
+    </controller>
+    <interface type='network'>
+      <mac address='52:54:00:7d:14:0e'/>
+      <source network='vagrant-libvirt'/>
+      <model type='virtio'/>
+      <address bus='0x00' domain='0x0000' function='0x0' slot='0x05' type='pci'/>
+    </interface>
+    <serial type='pty'>
+      <target port='0' type='isa-serial'>
+        <model name='isa-serial'/>
+      </target>
+    </serial>
+    <console type='pty'>
+      <target port='0' type='serial'/>
+    </console>
+    <input bus='ps2' type='mouse'/>
+    <input bus='ps2' type='keyboard'/>
+    <graphics autoport='yes' keymap='en-us' listen='127.0.0.1' port='-1' type='vnc'>
+      <listen address='127.0.0.1' type='address'/>
+    </graphics>
+    <audio id='1' type='none'/>
+    <video>
+      <model heads='1' primary='yes' type='cirrus' vram='16384'/>
+      <address bus='0x00' domain='0x0000' function='0x0' slot='0x02' type='pci'/>
+    </video>
+    <memballoon model='virtio'>
+      <address bus='0x00' domain='0x0000' function='0x0' slot='0x04' type='pci'/>
+    </memballoon>
+  </devices>
+</domain>