Use equivalent-xml to compare xml (#1564)

Using diffy alone is insufficient to spot when the XML returned by
libvirt for the changes applied is equivalent but formatted differently.

When libvirt returns slightly differently formatted XML corresponding to
what changes were actually applied, it can result in an error being
assumed to have happened when the update actually worked as expected.

Equivalent-xml handles detecting XML documents as being identical,
therefore switching to using it instead of diffy should produce more
reliable results, while retaining diffy for better formatted diffs
should an issue occur is useful.

Fixes: #1556

lib/vagrant-libvirt/action/start_domain.rb

@@ -1,7 +1,8 @@
 # frozen_string_literal: true
 
-require 'diffy'
 require 'log4r'
+
+require 'equivalent-xml'
 require 'rexml/document'
 
 module VagrantPlugins
@@ -425,24 +426,29 @@ module VagrantPlugins
                 end
 
                 begin
+                  # need to check whether the updated XML contains all the changes requested
+
+                  # This normalizes the attribute order to be consistent across both XML docs to
+                  # eliminate differences for subsequent comparison by diffy
+                  applied_xml_descr = REXML::Document.new(libvirt_domain.xml_desc(1))
+                  applied_xml = String.new
+                  applied_xml_descr.write(applied_xml)
+
                   proposed = Nokogiri::XML(new_xml, &:noblanks)
+                  applied = Nokogiri::XML(applied_xml, &:noblanks)
 
-                  # This normalizes the attribute order to be consistent across both XML docs to eliminate differences
-                  # for subsequent comparison by diffy
-                  updated_xml_descr = REXML::Document.new(libvirt_domain.xml_desc(1))
-                  updated_xml = String.new
-                  updated_xml_descr.write(updated_xml)
+                  if !EquivalentXml.equivalent?(proposed, applied)
+                    require 'diffy'
 
-                  updated = Nokogiri::XML(updated_xml, &:noblanks)
+                    # pretty print the XML as even though there can be additional changes,
+                    # the output with diffy appears to be clearer
+                    pretty_proposed = StringIO.new
+                    pretty_applied = StringIO.new
+                    proposed.write_xml_to(pretty_proposed, indent: 2)
+                    applied.write_xml_to(pretty_applied, indent: 2)
 
-                  pretty_proposed = StringIO.new
-                  pretty_updated = StringIO.new
-                  proposed.write_xml_to(pretty_proposed, indent: 2)
-                  updated.write_xml_to(pretty_updated, indent: 2)
+                    diff = Diffy::Diff.new(pretty_proposed.string, pretty_applied.string, :context => 3).to_s(:text)
 
-                  diff = Diffy::Diff.new(pretty_proposed.string, pretty_updated.string, :context => 3).to_s(:text)
-
-                  unless diff.empty?
                     error_msg = "Libvirt failed to fully update the domain with the specified XML. Result differs from requested:\n" +
                       "--- requested\n+++ result\n#{diff}\n" +
                       "Typically this means there is a bug in the XML being sent, please log an issue"

spec/unit/action/start_domain_spec.rb

@@ -50,6 +50,31 @@ describe VagrantPlugins::ProviderLibvirt::Action::StartDomain do
       expect(subject.call(env)).to be_nil
     end
 
+    context 'when xml is formatted differently' do
+      let(:test_file) { 'default_with_different_formatting.xml' }
+      let(:updated_domain_xml) {
+        new_xml = domain_xml.dup
+        new_xml.gsub!(/<cpu .*<\/cpu>/m, '<cpu check="none" mode="host-passthrough"/>')
+        new_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(:error)
+        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 any setting changed' do
       let(:vagrantfile_providerconfig) do
         <<-EOF

spec/unit/action/start_domain_spec/default_with_different_formatting.xml

@@ -0,0 +1,37 @@
+<domain xmlns:qemu='http://libvirt.org/schemas/domain/qemu/1.0' type=''>
+  <name/>
+  <title/>
+  <description/>
+  <uuid/>
+  <memory/>
+  <vcpu>1</vcpu>
+  <cpu check="none" mode="host-model">
+
+
+  </cpu>
+  <os>
+    <type>hvm</type>
+    <kernel/>
+    <initrd/>
+    <cmdline/>
+  </os>
+  <features>
+    <acpi/>
+    <apic/>
+    <pae/>
+  </features>
+  <clock offset='utc'/>
+  <devices>
+    <serial type='pty'>
+      <target port='0'/>
+    </serial>
+    <console type='pty'>
+      <target port='0'/>
+    </console>
+    <input bus='ps2' type='mouse'/>
+    <graphics autoport='yes' keymap='en-us' listen='127.0.0.1' port='-1' type='vnc'/>
+    <video>
+      <model heads='1' type='cirrus' vram='16384'/>
+    </video>
+  </devices>
+</domain>

vagrant-libvirt.gemspec

@@ -23,6 +23,7 @@ Gem::Specification.new do |s|
   s.add_runtime_dependency 'fog-libvirt', '>= 0.6.0'
   s.add_runtime_dependency 'fog-core', '~> 2'
   s.add_runtime_dependency 'rexml'
+  s.add_runtime_dependency 'equivalent-xml'
   s.add_runtime_dependency 'diffy'
 
   # Make sure to allow use of the same version as Vagrant by being less specific