Handle nic model type updating during start (#1747)

Start domain added some support to update the nic model type if it
changed to avoid needing to completely destroy and recreate the domain.
Additionally support for setting the management network nic model type
was added, however the default value did not correctly reflect expected.

This updates `management_network_model_type` to default to
`nic_model_type`, which in turn defaults to `virtio`. This has now been
moved from the create network interfaces action into the config object
to allow chained resolving. This should ensure that setting
`nic_model_type` will correctly cause all networks to use the model
unless explicitly specified to use a different model.

Additionally the start domain action for updating the interfaces has
been modified to loop over all interfaces and all configured adapters in
the order they should have been added. This allows for checking whether
the model type needs to be updated based on the
`management_network_model_type` config option for the first network,
assuming the management network has been attached, and otherwise to fall
back to the default from `nic_model_type` for all other interfaces.

Finally ensure the iommu attribute is removed from the nic driver if
the model type is switched to anything other virtio to avoid issues with
attempting to enable invalid settings.

Fixes: #1743
This commit is contained in:
Darragh Bailey
2023-06-20 15:46:05 +01:00
committed by GitHub
parent df1dd25e1e
commit 889b13d636
16 changed files with 549 additions and 110 deletions

View File

@@ -162,6 +162,109 @@ describe VagrantPlugins::ProviderLibvirt::Action::StartDomain do
end
end
context 'interface' do
let(:test_file) { 'existing_with_iommu.xml' }
let(:updated_domain_xml) {
new_xml = domain_xml.dup
new_xml.sub!(
/<model type='virtio'\/>\s+<driver iommu='on'\/>/m,
<<-EOF
<model type='e1000'/>
EOF
)
new_xml
}
let(:vagrantfile_providerconfig) {
<<-EOF
libvirt.management_network_model_type = 'e1000'
EOF
}
it 'should remove iommu if not interface model not virtio' do
expect(ui).to_not receive(:warn)
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(libvirt_domain).to receive(:autostart=)
expect(domain).to receive(:start)
expect(subject.call(env)).to be_nil
end
context 'iommu mismatch' do
let(:updated_domain_xml) {
new_xml = domain_xml.dup
new_xml.sub!(/(<model type='virtio'\/>\s+)<driver iommu='on'\/>/m) { |_|
match = Regexp.last_match
"#{match[1]}<driver iommu='off'/>"
}
new_xml
}
let(:vagrantfile_providerconfig) {
<<-EOF
libvirt.management_network_driver_iommu = false
EOF
}
it 'should update iommu to off' do
expect(ui).to_not receive(:warn)
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(libvirt_domain).to receive(:autostart=)
expect(domain).to receive(:start)
expect(subject.call(env)).to be_nil
end
end
context 'with additional interface' do
let(:test_file) { 'existing_with_two_interfaces_iommu.xml' }
let(:adapters) {
[
{:iface_type => :private_network, :model_type => "e1000", :network_name => "vagrant-libvirt", :driver_iommu => false},
{:iface_type => :private_network, :model_type => "virtio", :network_name => "vagrant-libvirt-1", :driver_iommu => true},
]
}
before do
allow(subject).to receive(:network_interfaces).and_return(adapters)
end
it 'should only update the management interface' do
expect(updated_domain_xml).to match(/<source network='vagrant-libvirt'\/>\s+<model type='e1000'\/>/m)
expect(updated_domain_xml).to match(/<source network='private'\/>\s+<model type='virtio'\/>/m)
expect(ui).to_not receive(:warn)
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(libvirt_domain).to receive(:autostart=)
expect(domain).to receive(:start)
expect(subject.call(env)).to be_nil
end
context 'with more adapters configured than attached' do
let(:adapters) {
[
{:iface_type => :private_network, :model_type => "e1000", :network_name => "vagrant-libvirt", :driver_iommu => false},
{:iface_type => :private_network, :model_type => "virtio", :network_name => "vagrant-libvirt-1", :driver_iommu => true},
{:iface_type => :private_network, :model_type => "virtio", :network_name => "vagrant-libvirt-2", :driver_iommu => true},
]
}
it 'should update and trigger a warning about mismatched adapters' do
expect(ui).to receive(:warn).with(/number of network adapters in current config \(3\) is different to attached interfaces \(2\)/)
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(libvirt_domain).to receive(:autostart=)
expect(domain).to receive(:start)
expect(subject.call(env)).to be_nil
end
end
end
end
context 'cpu' do
let(:test_file) { 'existing.xml' }
let(:updated_domain_xml) {

View File

@@ -23,6 +23,12 @@
</clock>
<devices>
<interface type='network'>
<alias name='ua-net-0'/>
<source network='vagrant-libvirt'/>
<model type='virtio'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/>
</interface>
<serial type='pty'>
<target port='0'/>
</serial>

View File

@@ -23,6 +23,12 @@
<timer name='rtc'/>
</clock>
<devices>
<interface type='network'>
<alias name='ua-net-0'/>
<source network='vagrant-libvirt'/>
<model type='virtio'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/>
</interface>
<serial type='pty'>
<target port='0'/>
</serial>

View File

@@ -24,6 +24,12 @@
<timer name='tsc'/>
</clock>
<devices>
<interface type='network'>
<alias name='ua-net-0'/>
<source network='vagrant-libvirt'/>
<model type='virtio'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/>
</interface>
<serial type='pty'>
<target port='0'/>
</serial>

View File

@@ -21,6 +21,12 @@
</features>
<clock offset='utc'/>
<devices>
<interface type='network'>
<alias name='ua-net-0'/>
<source network='vagrant-libvirt'/>
<model type='virtio'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/>
</interface>
<serial type='pty'>
<target port='0'/>
</serial>

View File

@@ -21,6 +21,12 @@
</features>
<clock offset='utc'/>
<devices>
<interface type='network'>
<alias name='ua-net-0'/>
<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'/>
</serial>

View File

@@ -21,6 +21,12 @@
</features>
<clock offset='utc'/>
<devices>
<interface type='network'>
<alias name='ua-net-0'/>
<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'/>
</serial>

View File

@@ -22,6 +22,12 @@
</features>
<clock offset='utc'/>
<devices>
<interface type='network'>
<alias name='ua-net-0'/>
<source network='vagrant-libvirt'/>
<model type='virtio'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/>
</interface>
<serial type='pty'>
<target port='0'/>
</serial>

View File

@@ -0,0 +1,64 @@
<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>
<bootmenu enable='no'/>
<boot dev='hd'/>
</os>
<features>
<acpi/>
<apic/>
<pae/>
</features>
<cpu mode='host-model' check='partial'/>
<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 type='file' device='disk'>
<driver name='qemu' type='qcow2'/>
<source file='/var/lib/libvirt/images/vagrant-libvirt_default.img'/>
<target dev='vda' bus='virtio'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
</disk>
<controller type='usb' index='0' model='piix3-uhci'>
<address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
</controller>
<controller type='pci' index='0' model='pci-root'/>
<interface type='network'>
<alias name='ua-net-0'/>
<source network='vagrant-libvirt'/>
<model type='virtio'/>
<driver iommu='on'/>
<address bus='0x00' domain='0x0000' function='0x0' slot='0x05' type='pci'/>
</interface>
<serial type='pty'>
<target type='isa-serial' port='0'>
<model name='isa-serial'/>
</target>
</serial>
<console type='pty'>
<target type='serial' port='0'/>
</console>
<input type='mouse' bus='ps2'/>
<input type='keyboard' bus='ps2'/>
<graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' keymap='en-us' websocket='-1'>
<listen type='address' address='127.0.0.1'/>
</graphics>
<audio id='1' type='none'/>
<video>
<model type='cirrus' vram='16384' heads='1' primary='yes'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
</video>
<memballoon model='virtio'>
<address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
</memballoon>
</devices>
</domain>

View File

@@ -0,0 +1,71 @@
<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>
<bootmenu enable='no'/>
<boot dev='hd'/>
</os>
<features>
<acpi/>
<apic/>
<pae/>
</features>
<cpu mode='host-model' check='partial'/>
<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 type='file' device='disk'>
<driver name='qemu' type='qcow2'/>
<source file='/var/lib/libvirt/images/vagrant-libvirt_default.img'/>
<target dev='vda' bus='virtio'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
</disk>
<controller type='usb' index='0' model='piix3-uhci'>
<address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
</controller>
<controller type='pci' index='0' model='pci-root'/>
<interface type='network'>
<alias name='ua-net-0'/>
<source network='vagrant-libvirt'/>
<model type='virtio'/>
<driver iommu='on'/>
<address bus='0x00' domain='0x0000' function='0x0' slot='0x05' type='pci'/>
</interface>
<interface type='network'>
<alias name='ua-net-1'/>
<source network='private'/>
<model type='virtio'/>
<driver iommu='on'/>
<address bus='0x00' domain='0x0000' function='0x0' slot='0x06' type='pci'/>
</interface>
<serial type='pty'>
<target type='isa-serial' port='0'>
<model name='isa-serial'/>
</target>
</serial>
<console type='pty'>
<target type='serial' port='0'/>
</console>
<input type='mouse' bus='ps2'/>
<input type='keyboard' bus='ps2'/>
<graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' keymap='en-us' websocket='-1'>
<listen type='address' address='127.0.0.1'/>
</graphics>
<audio id='1' type='none'/>
<video>
<model type='cirrus' vram='16384' heads='1' primary='yes'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
</video>
<memballoon model='virtio'>
<address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
</memballoon>
</devices>
</domain>

View File

@@ -8,28 +8,29 @@ describe 'VagrantPlugins::ProviderLibvirt::Util::NetworkUtil' do
include_context 'libvirt'
subject do
Class.new {
Class.new do
include VagrantPlugins::ProviderLibvirt::Util::NetworkUtil
def initialize
@logger = Log4r::Logger.new('test-logger')
end
}.new
end.new
end
def create_libvirt_network(name, attrs={})
def create_libvirt_network(name, attrs = {})
default_attrs = {
:active? => true,
:autostart? => true,
active?: true,
autostart?: true
}
network_xml = File.read(File.join(File.dirname(__FILE__), File.basename(__FILE__, '.rb'), name + '.xml'))
network_xml = File.read(File.join(File.dirname(__FILE__), File.basename(__FILE__, '.rb'), "#{name}.xml"))
double = instance_double(::Libvirt::Network)
allow(double).to receive(:xml_desc).and_return(network_xml)
allow(double).to receive(:name).and_return(name)
xml = REXML::Document.new(network_xml)
bridge = REXML::XPath.first(xml, '/network/bridge')
default_attrs[:bridge_name] = !bridge.nil? ? bridge.attributes['name'] : Libvirt::Error.new("network #{name} does not have attribute bridge_name")
default_attrs[:bridge_name] =
!bridge.nil? ? bridge.attributes['name'] : Libvirt::Error.new("network #{name} does not have attribute bridge_name")
default_attrs.merge(attrs).each do |aname, avalue|
if avalue.is_a?(Exception)
@@ -50,10 +51,133 @@ describe 'VagrantPlugins::ProviderLibvirt::Util::NetworkUtil' do
expect(logger).to_not receive(:debug)
expect(driver).to receive(:list_all_networks).and_return([default_network, additional_network])
expect(subject.libvirt_networks(driver)).to match_array([
hash_including(:name => 'default'),
hash_including(:name => 'vagrant-libvirt'),
])
expect(subject.libvirt_networks(driver)).to match_array(
[
hash_including(name: 'default'),
hash_including(name: 'vagrant-libvirt')
]
)
end
end
describe '#network_interfaces' do
let(:configured_networks_all) do
[
{
iface_type: :private_network,
ip: '192.168.121.0',
netmask: '255.255.255.0',
network_name: 'vagrant-libvirt',
},
{
auto_correct: true,
iface_type: :forwarded_port,
},
{
iface_type: :private_network,
ip: '192.168.123.0',
netmask: '255.255.255.0',
network_name: 'vagrant-libvirt-1',
},
{
iface_type: :private_network,
ip: '192.168.124.0',
netmask: '255.255.255.0',
network_name: 'vagrant-libvirt-2',
},
]
end
let(:configured_networks) do
[
configured_networks_all[0]
]
end
before do
expect(subject).to receive(:configured_networks).with(machine, logger).and_return(configured_networks)
end
it 'should return a list of default adapters configured' do
expect(logger).to receive(:debug).with('Adapter not specified so found slot 0')
expect(logger).to receive(:debug).with('Found network by name')
expect(subject.network_interfaces(machine, logger)).to match_array([configured_networks[0]])
end
context 'with forwarded ports' do
let(:configured_networks) do
[
configured_networks_all[0],
configured_networks_all[1]
]
end
it 'should skip the forwarded port' do
expect(logger).to receive(:debug).with('Adapter not specified so found slot 0')
expect(logger).to receive(:debug).with('Found network by name')
expect(subject.network_interfaces(machine, logger)).to match_array([configured_networks[0]])
end
end
context 'with 2 additional private networks with adapter set' do
let(:configured_networks) do
[
configured_networks_all[0],
configured_networks_all[2].merge(:adapter => 2),
configured_networks_all[3],
]
end
it 'should return the first private network last' do
expect(logger).to receive(:debug).with('Adapter not specified so found slot 0')
expect(logger).to receive(:debug).with('Found network by name').exactly(3).times
expect(logger).to receive(:debug).with('Using specified adapter slot 2')
expect(logger).to receive(:debug).with('Adapter not specified so found slot 1')
expect(subject.network_interfaces(machine, logger)).to match_array(
[
configured_networks[0],
configured_networks[2],
configured_networks[1]
]
)
end
end
end
describe '#configured_networks' do
it 'should return a list of default adapters configured' do
expect(logger).to receive(:info).with('Using vagrant-libvirt at 192.168.121.0/24 as the management network nat is the mode')
expect(logger).to receive(:debug).with(/In config found network type forwarded_port options/)
expect(subject.configured_networks(machine, logger)).to match_array(
[
hash_including(
{
forward_mode: 'nat',
iface_type: :private_network,
ip: '192.168.121.0',
model_type: 'virtio',
netmask: '255.255.255.0',
network_name: 'vagrant-libvirt',
}
),
hash_including(
{
auto_correct: true,
forward_mode: 'nat',
guest: 22,
host: 2222,
host_ip: '127.0.0.1',
id: 'ssh',
iface_type: :forwarded_port,
netmask: '255.255.255.0',
protocol: 'tcp',
}
)
]
)
end
end
end