NuKVM/vagrant-libvirt
3
0
Fork 0
mirror of https://github.com/vagrant-libvirt/vagrant-libvirt.git synced 2025-02-25 18:55:27 -06:00
Code Issues Packages Projects Releases Wiki Activity

Add action to resolve disk settings (#1502)

Browse Source 
With multi volume boxes, need to ensure that disk settings such as the
device assigned are resolved dynamically once it has been established
which devices have already been assigned to the box volumes on either
initial creation or subsequent boots.

Otherwise users are forced to always explicitly define the device for
additional storage instead of having it be automatically assigned the
next available device.

Consequently previous changes have broken the ability for machines
with additional storage to be halted and restarted correctly.

Include an integration test that for additional storage checks that the
machine can be stopped and started again.

Fixes: #1490
This commit is contained in:
Darragh Bailey
2022-06-02 19:09:18 +01:00
committed by GitHub
parent d1232f0c20
commit 37597e22f9
20 changed files with 986 additions and 256 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
lib/vagrant-libvirt/action.rb
View File

@@ -35,6 +35,7 @@ module VagrantPlugins
autoload :ReadMacAddresses, action_root.join('read_mac_addresses')
autoload :RemoveLibvirtImage, action_root.join('remove_libvirt_image')
autoload :RemoveStaleVolume, action_root.join('remove_stale_volume')
autoload :ResolveDiskSettings, action_root.join('resolve_disk_settings')
autoload :ResumeDomain, action_root.join('resume_domain')
autoload :SetNameOfDomain, action_root.join('set_name_of_domain')
autoload :SetBootOrder, action_root.join('set_boot_order')
@@ -81,6 +82,7 @@ module VagrantPlugins
b2.use SetNameOfDomain
if !env[:machine].config.vm.box
b2.use ResolveDiskSettings
b2.use CreateDomain
b2.use CreateNetworks
b2.use CreateNetworkInterfaces
@@ -94,6 +96,7 @@ module VagrantPlugins
b2.use HandleBox
b2.use HandleBoxImage
b2.use CreateDomainVolume
b2.use ResolveDiskSettings
b2.use CreateDomain
b2.use CreateNetworks
b2.use CreateNetworkInterfaces
@@ -104,6 +107,7 @@ module VagrantPlugins
end
else
env[:halt_on_error] = true
b2.use ResolveDiskSettings
b2.use CreateNetworks
b2.use action_start
end

110
lib/vagrant-libvirt/action/create_domain.rb
View File

@@ -16,20 +16,6 @@ module VagrantPlugins
@app = app
end
def _disk_name(name, disk)
"#{name}-#{disk[:device]}.#{disk[:type]}" # disk name
end
def _disks_print(disks)
disks.collect do |x|
"#{x[:device]}(#{x[:type]}, #{x[:bus]}, #{x[:size]})"
end.join(', ')
end
def _cdroms_print(cdroms)
cdroms.collect { |x| x[:dev] }.join(', ')
end
def call(env)
# Get config.
config = env[:machine].provider_config
@@ -58,8 +44,6 @@ module VagrantPlugins
@nvram = config.nvram
@machine_type = config.machine_type
@machine_arch = config.machine_arch
@disk_bus = config.disk_bus
@disk_device = config.disk_device
@disk_driver_opts = config.disk_driver_opts
@nested = config.nested
@memory_size = config.memory.to_i * 1024
@@ -94,9 +78,8 @@ module VagrantPlugins
# Storage
@storage_pool_name = config.storage_pool_name
@snapshot_pool_name = config.snapshot_pool_name
@domain_volumes = []
@disks = config.disks
@domain_volumes = env[:domain_volumes] || []
@disks = env[:disks] || []
@cdroms = config.cdroms
# Input
@@ -139,77 +122,19 @@ module VagrantPlugins
@memballoon_pci_bus = config.memballoon_pci_bus
@memballoon_pci_slot = config.memballoon_pci_slot
config = env[:machine].provider_config
@domain_type = config.driver
@os_type = 'hvm'
resolver = ::VagrantPlugins::ProviderLibvirt::Util::DiskDeviceResolver.new(prefix=@disk_device[0..1])
# Get path to domain image from the storage pool selected if we have a box.
if env[:machine].config.vm.box
if @snapshot_pool_name != @storage_pool_name
pool_name = @snapshot_pool_name
else
pool_name = @storage_pool_name
end
# special handling for domain volume
env[:box_volumes][0][:device] = env[:box_volumes][0].fetch(:device, @disk_device)
resolver.resolve!(env[:box_volumes])
@logger.debug "Search for volumes in pool: #{pool_name}"
env[:box_volumes].each_index do |index|
suffix_index = index > 0 ? "_#{index}" : ''
domain_volume = env[:machine].provider.driver.connection.volumes.all(
name: "#{@name}#{suffix_index}.img"
).find { |x| x.pool_name == pool_name }
raise Errors::DomainVolumeExists if domain_volume.nil?
@domain_volumes.push({
:dev => env[:box_volumes][index][:device],
:cache => @domain_volume_cache,
:bus => @disk_bus,
:path => domain_volume.path,
:virtual_size => env[:box_volumes][index][:virtual_size]
})
end
env[:domain_volumes].each_with_index do |vol, index|
suffix_index = index > 0 ? "_#{index}" : ''
domain_volume = env[:machine].provider.driver.connection.volumes.all(
name: "#{@name}#{suffix_index}.img"
).find { |x| x.pool_name == vol[:pool] }
raise Errors::NoDomainVolume if domain_volume.nil?
end
# If we have a box, take the path from the domain volume and set our storage_prefix.
# If not, we dump the storage pool xml to get its defined path.
# the default storage prefix is typically: /var/lib/libvirt/images/
if env[:machine].config.vm.box
storage_prefix = File.dirname(@domain_volumes[0][:path]) + '/' # steal
else
storage_prefix = get_disk_storage_prefix(env, @storage_pool_name)
end
resolver.resolve!(@disks)
@disks.each do |disk|
disk[:path] ||= _disk_name(@name, disk)
# On volume creation, the <path> element inside <target>
# is oddly ignored; instead the path is taken from the
# <name> element:
# http://www.redhat.com/archives/libvir-list/2008-August/msg00329.html
disk[:name] = disk[:path]
disk[:absolute_path] = storage_prefix + disk[:path]
if not disk[:pool].nil?
disk_pool_name = disk[:pool]
@logger.debug "Overriding pool name with: #{disk_pool_name}"
disk_storage_prefix = get_disk_storage_prefix(env, disk_pool_name)
disk[:absolute_path] = disk_storage_prefix + disk[:path]
@logger.debug "Overriding disk path with: #{disk[:absolute_path]}"
else
disk_pool_name = @storage_pool_name
end
# make the disk. equivalent to:
# qemu-img create -f qcow2 <path> 5g
begin
@@ -221,13 +146,13 @@ module VagrantPlugins
owner: storage_uid(env),
group: storage_gid(env),
#:allocation => ?,
pool_name: disk_pool_name
pool_name: disk[:pool],
)
rescue Libvirt::Error => e
# It is hard to believe that e contains just a string
# and no useful error code!
msg = "Call to virStorageVolCreateXML failed: " +
"storage volume '#{disk[:path]}' exists already"
"storage volume '#{disk[:absolute_path]}' exists already"
if e.message == msg and disk[:allow_existing]
disk[:preexisting] = true
else
@@ -300,7 +225,7 @@ module VagrantPlugins
end
env[:ui].info(" -- Storage pool: #{@storage_pool_name}")
@domain_volumes.each do |volume|
env[:ui].info(" -- Image(#{volume[:dev]}): #{volume[:path]}, #{volume[:bus]}, #{volume[:virtual_size].to_GB}G")
env[:ui].info(" -- Image(#{volume[:device]}): #{volume[:absolute_path]}, #{volume[:bus]}, #{volume[:virtual_size].to_GB}G")
end
if not @disk_driver_opts.empty?
@@ -466,11 +391,14 @@ module VagrantPlugins
end
private
def get_disk_storage_prefix(env, disk_pool_name)
disk_storage_pool = env[:machine].provider.driver.connection.client.lookup_storage_pool_by_name(disk_pool_name)
raise Errors::NoStoragePool if disk_storage_pool.nil?
xml = Nokogiri::XML(disk_storage_pool.xml_desc)
disk_storage_prefix = xml.xpath('/pool/target/path').inner_text.to_s + '/'
def _disks_print(disks)
disks.collect do |x|
"#{x[:device]}(#{x[:type]}, #{x[:bus]}, #{x[:size]})"
end.join(', ')
end
def _cdroms_print(cdroms)
cdroms.collect { |x| x[:dev] }.join(', ')
end
end
end

2
lib/vagrant-libvirt/action/destroy_domain.rb
View File

@@ -62,7 +62,7 @@ module VagrantPlugins
disks_xml = REXML::XPath.match(xml_descr, '/domain/devices/disk[@device="disk"]')
have_aliases = !(REXML::XPath.match(disks_xml, './alias[@name="ua-box-volume-0"]').first).nil?
if !have_aliases
env[:ui].warn(I18n.t('vagrant_libvirt.destroy.obsolete_method'))
env[:ui].warn(I18n.t('vagrant_libvirt.domain_xml.obsolete_method'))
end
destroy_domain(domain, destroy_volumes: false, flags: undefine_flags)

2
lib/vagrant-libvirt/action/handle_box_image.rb
View File

@@ -104,6 +104,8 @@ module VagrantPlugins
end
# save for use by later actions
env[:box_volumes][0][:virtual_size] = box_virtual_size
# special handling for domain volume
env[:box_volumes][0][:device] ||= config.disk_device
# while inside the synchronize block take care not to call the next
# action in the chain, as must exit this block first to prevent

174
lib/vagrant-libvirt/action/resolve_disk_settings.rb Normal file
View File

@@ -0,0 +1,174 @@
# frozen_string_literal: true
require 'log4r'
require 'rexml'
require 'vagrant-libvirt/util/resolvers'
module VagrantPlugins
module ProviderLibvirt
module Action
class ResolveDiskSettings
def initialize(app, _env)
@logger = Log4r::Logger.new('vagrant_libvirt::action::resolve_disk_devices')
@app = app
end
def call(env)
# Get config.
config = env[:machine].provider_config
domain_name = env[:domain_name] # only set on create
disk_bus = config.disk_bus
disk_device = config.disk_device
domain_volume_cache = config.volume_cache || 'default'
# Storage
storage_pool_name = config.storage_pool_name
snapshot_pool_name = config.snapshot_pool_name
domain_volumes = []
disks = config.disks.dup
resolver = ::VagrantPlugins::ProviderLibvirt::Util::DiskDeviceResolver.new(disk_device[0..1])
# Get path to domain image from the storage pool selected if we have a box.
if env[:machine].config.vm.box
pool_name = if snapshot_pool_name == storage_pool_name
storage_pool_name
else
snapshot_pool_name
end
if env[:box_volumes].nil?
# domain must be already created, need to read domain volumes from domain XML
libvirt_domain = env[:machine].provider.driver.connection.client.lookup_domain_by_uuid(
env[:machine].id
)
domain_xml = libvirt_domain.xml_desc(1)
xml_descr = REXML::Document.new(domain_xml)
domain_name = xml_descr.elements['domain'].elements['name'].text
disks_xml = REXML::XPath.match(xml_descr, '/domain/devices/disk[@device="disk"]')
have_aliases = !REXML::XPath.match(disks_xml, './alias[@name="ua-box-volume-0"]').first.nil?
env[:ui].warn(I18n.t('vagrant_libvirt.domain_xml.obsolete_method')) unless have_aliases
if have_aliases
REXML::XPath.match(disks_xml,
'./alias[contains(@name, "ua-box-volume-")]').each_with_index do |alias_xml, idx|
domain_volumes.push(volume_from_xml(alias_xml.parent, domain_name, idx))
end
else
# fallback to try and infer which boxes are box images, as they are listed first
# as soon as there is no match, can exit
disks_xml.each_with_index do |box_disk_xml, idx|
diskname = box_disk_xml.elements['source'].attributes['file'].rpartition('/').last
break if volume_name(domain_name, idx) != diskname
domain_volumes.push(volume_from_xml(box_disk_xml, domain_name, idx))
end
end
else
@logger.debug "Search for volumes in pool: #{pool_name}"
env[:box_volumes].each_index do |index|
domain_volume = env[:machine].provider.driver.connection.volumes.all(
name: volume_name(domain_name, index)
).find { |x| x.pool_name == pool_name }
raise Errors::NoDomainVolume if domain_volume.nil?
domain_volumes.push(
{
name: volume_name(domain_name, index),
device: env[:box_volumes][index][:device],
cache: domain_volume_cache,
bus: disk_bus,
absolute_path: domain_volume.path,
virtual_size: env[:box_volumes][index][:virtual_size],
pool: pool_name,
}
)
end
end
resolver.resolve!(domain_volumes)
# If we have a box, take the path from the domain volume and set our storage_prefix.
# If not, we dump the storage pool xml to get its defined path.
# the default storage prefix is typically: /var/lib/libvirt/images/
storage_prefix = "#{File.dirname(domain_volumes[0][:absolute_path])}/" # steal
else
# Ensure domain name is set for subsequent steps if restarting a machine without a box
libvirt_domain = env[:machine].provider.driver.connection.client.lookup_domain_by_uuid(
env[:machine].id
)
domain_xml = libvirt_domain.xml_desc(1)
xml_descr = REXML::Document.new(domain_xml)
domain_name = xml_descr.elements['domain'].elements['name'].text
storage_prefix = get_disk_storage_prefix(env[:machine], storage_pool_name)
end
resolver.resolve!(disks)
disks.each do |disk|
disk[:path] ||= disk_name(domain_name, disk)
# On volume creation, the <path> element inside <target>
# is oddly ignored; instead the path is taken from the
# <name> element:
# http://www.redhat.com/archives/libvir-list/2008-August/msg00329.html
disk[:name] = disk[:path]
disk[:absolute_path] = storage_prefix + disk[:path]
if disk[:pool].nil?
disk[:pool] = storage_pool_name
else
@logger.debug "Overriding pool name with: #{disk[:pool]}"
disk_storage_prefix = get_disk_storage_prefix(env[:machine], disk[:pool])
disk[:absolute_path] = disk_storage_prefix + disk[:path]
@logger.debug "Overriding disk path with: #{disk[:absolute_path]}"
end
end
env[:domain_volumes] = domain_volumes
env[:disks] = disks
@app.call(env)
end
private
def disk_name(name, disk)
"#{name}-#{disk[:device]}.#{disk[:type]}" # disk name
end
def get_disk_storage_prefix(machine, disk_pool_name)
disk_storage_pool = machine.provider.driver.connection.client.lookup_storage_pool_by_name(disk_pool_name)
raise Errors::NoStoragePool if disk_storage_pool.nil?
xml = Nokogiri::XML(disk_storage_pool.xml_desc)
"#{xml.xpath('/pool/target/path').inner_text}/"
end
def volume_name(domain_name, index)
domain_name + (index.zero? ? '.img' : "_#{index}.img")
end
def volume_from_xml(device_xml, domain_name, index)
driver = device_xml.elements['driver']
source = device_xml.elements['source']
target = device_xml.elements['target']
{
name: volume_name(domain_name, index),
device: target.attributes['dev'],
cache: driver.attributes['cache'],
bus: target.attributes['bus'],
absolute_path: source.attributes['file'],
}
end
end
end
end
end

4
lib/vagrant-libvirt/templates/domain.xml.erb
View File

@@ -117,9 +117,9 @@
@disk_driver_opts.reject { |k,v| v.nil? }
.map { |k,v| "#{k}='#{v}'"}
.join(' ') -%>/>
<source file='<%= volume[:path] %>'/>
<source file='<%= volume[:absolute_path] %>'/>
<%# we need to ensure a unique target dev -%>
<target dev='<%= volume[:dev] %>' bus='<%= volume[:bus] %>'/>
<target dev='<%= volume[:device] %>' bus='<%= volume[:bus] %>'/>
</disk>
<%- end -%>
<%# additional disks -%>