Skip to content

Commit ec4daf3

Browse files
committed
Fix REXML 3.4.2+ compatibility
REXML 3.4.2+ deprecated accepting array as an element in `XPath.match` [[1]]. This led to test errors such as: ~~~ 3) VagrantPlugins::ProviderLibvirt::Action::ResolveDiskSettings#call when vm box is in use when box metadata is not available when multiple volumes in domain config should populate domain volumes with devices Failure/Error: expect(env[:domain_volumes]).to match( [ hash_including( device: 'vda', absolute_path: '/var/lib/libvirt/images/vagrant-test_default.img' ), hash_including( device: 'vdb', absolute_path: '/var/lib/libvirt/images/vagrant-test_default_1.img' ), expected [{absolute_path: "/var/lib/libvirt/images/vagrant-test_default.img", bus: "virtio", cache: "default", device: "vda", name: "vagrant-test_default.img"}] to match [#<RSpec::Mocks::ArgumentMatchers::HashIncludingMatcher:0x00007fff921b3db8 @expected={device: "vda", absolute_path: "/var/lib/libvirt/images/vagrant-test_default.img"}>, #<RSpec::Mocks::ArgumentMatchers::HashIncludingMatcher:0x00007fff921b3d40 @expected={device: "vdb", absolute_path: "/var/lib/libvirt/images/vagrant-test_default_1.img"}>, #<RSpec::Mocks::ArgumentMatchers::HashIncludingMatcher:0x00007fff921b3cc8 @expected={device: "vdc", absolute_path: "/var/lib/libvirt/images/vagrant-test_default_2.img"}>] Diff: @@ -1,4 +1,6 @@ -[hash_including(device: "vda", absolute_path: "/var/lib/libvirt/images/vagrant-test_default.img"), - hash_including(device: "vdb", absolute_path: "/var/lib/libvirt/images/vagrant-test_default_1.img"), - hash_including(device: "vdc", absolute_path: "/var/lib/libvirt/images/vagrant-test_default_2.img")] +[{absolute_path: "/var/lib/libvirt/images/vagrant-test_default.img", + bus: "virtio", + cache: "default", + device: "vda", + name: "vagrant-test_default.img"}] # ./spec/unit/action/resolve_disk_settings_spec.rb:200:in 'block (6 levels) in <top (required)>' # ./spec/support/unit_context.rb:51:in 'block (3 levels) in <top (required)>' # ./spec/support/unit_context.rb:43:in 'block (2 levels) in <top (required)>' # ./spec/support/unit_context.rb:51:in 'block (3 levels) in <top (required)>' # ./spec/support/unit_context.rb:43:in 'block (2 levels) in <top (required)>' ~~~ This changes the logic in a way, that XPath is matching against whole XML document, instead of array of XML elements. [1]: ruby/rexml#252
1 parent a94ce0d commit ec4daf3

File tree

2 files changed

+11
-8
lines changed

2 files changed

+11
-8
lines changed

lib/vagrant-libvirt/action/destroy_domain.rb

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ def call(env)
5959
domain_xml = libvirt_domain.xml_desc(1)
6060
xml_descr = REXML::Document.new(domain_xml)
6161
disks_xml = REXML::XPath.match(xml_descr, '/domain/devices/disk[@device="disk"]')
62-
have_aliases = !(REXML::XPath.match(disks_xml, './alias[@name="ua-box-volume-0"]').first).nil?
62+
have_aliases = !REXML::XPath.match(xml_descr, '/domain/devices/disk[@device="disk"]/alias[@name="ua-box-volume-0"]').first.nil?
6363
if !have_aliases
6464
env[:ui].warn(I18n.t('vagrant_libvirt.domain_xml.obsolete_method'))
6565
end
@@ -73,7 +73,9 @@ def call(env)
7373
# the additional storage devices are.
7474
detected_box_volumes = 0
7575
if have_aliases
76-
REXML::XPath.match(disks_xml, './alias[contains(@name, "ua-box-volume-")]').each do |box_disk|
76+
REXML::XPath.match(xml_descr,
77+
'/domain/devices/disk[@device="disk"]/alias[contains(@name, "ua-box-volume-")]'
78+
).each do |box_disk|
7779
diskname = box_disk.parent.elements['source'].attributes['file'].rpartition('/').last
7880
detected_box_volumes += 1
7981

@@ -130,13 +132,13 @@ def call(env)
130132
# look for exact match using aliases which will be used
131133
# for subsequent domain creations
132134
if have_aliases
133-
domain_disk = REXML::XPath.match(disks_xml, './alias[@name="ua-disk-volume-' + index.to_s + '"]').first
135+
domain_disk = REXML::XPath.match(xml_descr, '/domain/devices/disk[@device="disk"]/alias[@name="ua-disk-volume-' + index.to_s + '"]').first
134136
domain_disk = domain_disk.parent if !domain_disk.nil?
135137
else
136138
# otherwise fallback to find the disk by device if specified by user
137139
# and finally index counting with offset and hope the match is correct
138140
if !disk[:device].nil?
139-
domain_disk = REXML::XPath.match(disks_xml, './target[@dev="' + disk[:device] + '"]').first
141+
domain_disk = REXML::XPath.match(xml_descr, '/domain/devices/disk[@device="disk"]/target[@dev="' + disk[:device] + '"]').first
140142
domain_disk = domain_disk.parent if !domain_disk.nil?
141143
else
142144
domain_disk = disks_xml[offset + index]

lib/vagrant-libvirt/action/resolve_disk_settings.rb

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,18 +50,19 @@ def call(env)
5050
domain_xml = libvirt_domain.xml_desc(1)
5151
xml_descr = REXML::Document.new(domain_xml)
5252
domain_name = xml_descr.elements['domain'].elements['name'].text
53-
disks_xml = REXML::XPath.match(xml_descr, '/domain/devices/disk[@device="disk"]')
54-
have_aliases = !REXML::XPath.match(disks_xml, './alias[@name="ua-box-volume-0"]').first.nil?
53+
have_aliases = !REXML::XPath.match(xml_descr, '/domain/devices/disk[@device="disk"]/alias[@name="ua-box-volume-0"]').first.nil?
5554
env[:ui].warn(I18n.t('vagrant_libvirt.domain_xml.obsolete_method')) unless have_aliases
5655

5756
if have_aliases
58-
REXML::XPath.match(disks_xml,
59-
'./alias[contains(@name, "ua-box-volume-")]').each_with_index do |alias_xml, idx|
57+
REXML::XPath.match(xml_descr,
58+
'/domain/devices/disk[@device="disk"]/alias[contains(@name, "ua-box-volume-")]'
59+
).each_with_index do |alias_xml, idx|
6060
domain_volumes.push(volume_from_xml(alias_xml.parent, domain_name, idx))
6161
end
6262
else
6363
# fallback to try and infer which boxes are box images, as they are listed first
6464
# as soon as there is no match, can exit
65+
disks_xml = REXML::XPath.match(xml_descr, '/domain/devices/disk[@device="disk"]')
6566
disks_xml.each_with_index do |box_disk_xml, idx|
6667
diskname = box_disk_xml.elements['source'].attributes['file'].rpartition('/').last
6768

0 commit comments

Comments
 (0)