diff --git a/lib/vagrant-libvirt/action/start_domain.rb b/lib/vagrant-libvirt/action/start_domain.rb index 5a146ac..c796f67 100644 --- a/lib/vagrant-libvirt/action/start_domain.rb +++ b/lib/vagrant-libvirt/action/start_domain.rb @@ -2,14 +2,16 @@ require 'log4r' -require 'compare-xml' require 'rexml/document' +require 'vagrant-libvirt/util/xml' + module VagrantPlugins module ProviderLibvirt module Action # Just start the domain. class StartDomain + def initialize(app, _env) @logger = Log4r::Logger.new('vagrant_libvirt::action::start_domain') @app = app @@ -427,27 +429,13 @@ module VagrantPlugins begin # 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)) - # 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) - - if CompareXML.equivalent?(proposed, applied, { force_children: true }) + if proposed != applied require 'diffy' - # 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) - - diff = Diffy::Diff.new(pretty_proposed.string, pretty_applied.string, :context => 3).to_s(:text) + diff = Diffy::Diff.new(proposed, applied, :context => 3).to_s(:text) error_msg = "Libvirt failed to fully update the domain with the specified XML. Result differs from requested:\n" + "--- requested\n+++ result\n#{diff}\n" + diff --git a/lib/vagrant-libvirt/util/xml.rb b/lib/vagrant-libvirt/util/xml.rb new file mode 100644 index 0000000..e0e0e7e --- /dev/null +++ b/lib/vagrant-libvirt/util/xml.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +require 'xmlsimple' + +module VagrantPlugins + module ProviderLibvirt + module Util + class Xml + attr_reader :xml + + def initialize(xmlstr) + @xml = compact_content(XmlSimple.xml_in(xmlstr, {'NormaliseSpace' => 2})) + end + + def to_str + XmlSimple.xml_out(@xml) + end + + def ==(other) + @xml == other.xml + end + + private + + # content elements that are empty are preserved by xml-simple and will result + # in the structures being considered different even if functionality the same + # strip any empty ones to avoid. + def compact_content(node) + if node.is_a?(Array) + node.map! do |element| + compact_content(element) + end + elsif node.is_a?(Hash) + if node['content'] and node['content'].empty? + node.delete('content') + end + node.each do |k, v| + node[k] = compact_content(v) + end + else + return node + end + end + end + end + end +end diff --git a/spec/unit/action/start_domain_spec.rb b/spec/unit/action/start_domain_spec.rb index dd1bd84..aa5abcc 100644 --- a/spec/unit/action/start_domain_spec.rb +++ b/spec/unit/action/start_domain_spec.rb @@ -72,7 +72,30 @@ describe VagrantPlugins::ProviderLibvirt::Action::StartDomain do 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) { + new_xml = domain_xml.dup + new_xml.gsub!(//m, '') + new_xml + } + let(:vagrantfile_providerconfig) do + <<-EOF + libvirt.cpu_mode = "host-passthrough" + EOF + end + + it 'should error and revert the update' do + expect(ui).to receive(:error) + expect(connection).to receive(:define_domain).and_return(libvirt_domain) + expect(connection).to receive(:define_domain).with(domain_xml) # undo + expect(libvirt_domain).to receive(:xml_desc).and_return(domain_xml, updated_domain_xml) + expect(domain).to_not receive(:start) + + expect { subject.call(env) }.to raise_error(VagrantPlugins::ProviderLibvirt::Errors::FogError) + end end context 'when any setting changed' do diff --git a/vagrant-libvirt.gemspec b/vagrant-libvirt.gemspec index 8b052ee..457277f 100644 --- a/vagrant-libvirt.gemspec +++ b/vagrant-libvirt.gemspec @@ -23,7 +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 'compare-xml' + s.add_runtime_dependency 'xml-simple' s.add_runtime_dependency 'diffy' # Make sure to allow use of the same version as Vagrant by being less specific