Skip to content

Commit

Permalink
Handle different attribute and element ordering (#1592)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
electrofelix committed Sep 22, 2022
1 parent 5e0b169 commit f111842
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 1 deletion.
13 changes: 12 additions & 1 deletion lib/vagrant-libvirt/action/start_domain.rb
Expand Up @@ -429,9 +429,20 @@ def call(env)
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'
Expand Down
4 changes: 4 additions & 0 deletions spec/spec_helper.rb
Expand Up @@ -63,6 +63,10 @@ def branch_coverage?

# 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'
Expand Down
20 changes: 20 additions & 0 deletions spec/unit/action/start_domain_spec.rb
Expand Up @@ -74,6 +74,26 @@
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) {
Expand Down
62 changes: 62 additions & 0 deletions 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>

0 comments on commit f111842

Please sign in to comment.