Improve handling of cpu model and initrd settings (#1333)

Avoid triggering a reconfig of the domain when the cpu model and the
initrd haven't been set via the config.

lib/vagrant-libvirt/action/start_domain.rb

@@ -101,11 +101,13 @@ module VagrantPlugins
               if config.cpu_mode != 'host-passthrough'
                 cpu_model = REXML::XPath.first(xml_descr, '/domain/cpu/model')
                 if cpu_model.nil?
-                  @logger.debug "cpu_model updated from not set to '#{config.cpu_model}'"
-                  descr_changed = true
-                  cpu_model = REXML::Element.new('model', REXML::XPath.first(xml_descr, '/domain/cpu'))
-                  cpu_model.attributes['fallback'] = 'allow'
-                  cpu_model.text = config.cpu_model
+                  if config.cpu_model.strip != ''
+                    @logger.debug "cpu_model updated from not set to '#{config.cpu_model}'"
+                    descr_changed = true
+                    cpu_model = REXML::Element.new('model', REXML::XPath.first(xml_descr, '/domain/cpu'))
+                    cpu_model.attributes['fallback'] = 'allow'
+                    cpu_model.text = config.cpu_model
+                  end
                 else
                   if (cpu_model.text or '').strip != config.cpu_model.strip
                     @logger.debug "cpu_model text updated from #{cpu_model.text} to '#{config.cpu_model}'"
@@ -322,10 +324,12 @@ module VagrantPlugins
               if config.initrd
                 initrd = REXML::XPath.first(xml_descr, '/domain/os/initrd')
                 if initrd.nil?
-                  @logger.debug "initrd updated from not set to '#{config.initrd}'"
-                  descr_changed = true
-                  initrd = REXML::Element.new('initrd', REXML::XPath.first(xml_descr, '/domain/os'))
-                  initrd.text = config.initrd
+                  if config.initrd.strip != ''
+                    @logger.debug "initrd updated from not set to '#{config.initrd}'"
+                    descr_changed = true
+                    initrd = REXML::Element.new('initrd', REXML::XPath.first(xml_descr, '/domain/os'))
+                    initrd.text = config.initrd
+                  end
                 else
                   if (initrd.text or '').strip != config.initrd
                     @logger.debug "initrd updated from '#{initrd.text}' to '#{config.initrd}'"

spec/unit/action/start_domain_spec.rb

@@ -21,6 +21,8 @@ describe VagrantPlugins::ProviderLibvirt::Action::StartDomain do
   let(:updated_domain_xml) { File.read(File.join(File.dirname(__FILE__), File.basename(__FILE__, '.rb'), updated_test_file)) }
 
   describe '#call' do
+    let(:test_file) { 'default.xml' }
+
     before do
       allow_any_instance_of(VagrantPlugins::ProviderLibvirt::Driver)
         .to receive(:connection).and_return(connection)
@@ -33,17 +35,23 @@ describe VagrantPlugins::ProviderLibvirt::Action::StartDomain do
       allow(logger).to receive(:debug)
       expect(logger).to receive(:info)
       expect(ui).to_not receive(:error)
+
+      allow(libvirt_domain).to receive(:xml_desc).and_return(domain_xml)
+
+      allow(libvirt_domain).to receive(:max_memory).and_return(512*1024)
+      allow(libvirt_domain).to receive(:num_vcpus).and_return(1)
     end
 
-    context 'default config' do
-      let(:test_file) { 'default.xml' }
+    it 'should execute without changing' do
+      allow(libvirt_domain).to receive(:undefine)
+      expect(libvirt_domain).to receive(:autostart=)
+      expect(domain).to receive(:start)
 
-      before do
-        allow(libvirt_domain).to receive(:xml_desc).and_return(domain_xml)
+      expect(subject.call(env)).to be_nil
+    end
 
-        allow(libvirt_domain).to receive(:max_memory).and_return(512*1024)
-        allow(libvirt_domain).to receive(:num_vcpus).and_return(1)
-      end
+    context 'when previously running default config' do
+      let(:test_file) { 'existing.xml' }
 
       it 'should execute without changing' do
         allow(libvirt_domain).to receive(:undefine)
@@ -55,15 +63,6 @@ describe VagrantPlugins::ProviderLibvirt::Action::StartDomain do
     end
 
     context 'tpm' do
-      let(:test_file) { 'default.xml' }
-
-      before do
-        allow(libvirt_domain).to receive(:xml_desc).and_return(domain_xml)
-
-        allow(libvirt_domain).to receive(:max_memory).and_return(512*1024)
-        allow(libvirt_domain).to receive(:num_vcpus).and_return(1)
-      end
-
       context 'passthrough tpm added' do
         let(:updated_test_file) { 'default_added_tpm_path.xml' }
         let(:vagrantfile_providerconfig) do
@@ -170,13 +169,6 @@ describe VagrantPlugins::ProviderLibvirt::Action::StartDomain do
     context 'clock_timers' do
       let(:test_file) { 'clock_timer_rtc.xml' }
 
-      before do
-        allow(libvirt_domain).to receive(:xml_desc).and_return(domain_xml)
-
-        allow(libvirt_domain).to receive(:max_memory).and_return(512*1024)
-        allow(libvirt_domain).to receive(:num_vcpus).and_return(1)
-      end
-
       context 'timers unchanged' do
         let(:vagrantfile_providerconfig) do
           <<-EOF
@@ -214,8 +206,6 @@ describe VagrantPlugins::ProviderLibvirt::Action::StartDomain do
       end
 
       context 'timers removed' do
-        let(:updated_test_file) { 'default.xml' }
-
         it 'should modify the domain' do
           expect(libvirt_domain).to receive(:undefine)
           expect(logger).to receive(:debug).with('clock timers config changed')

spec/unit/action/start_domain_spec/existing.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 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'>
+      <mac address='52:54:00:7d:14:0e'/>
+      <source network='vagrant-libvirt'/>
+      <model type='virtio'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/>
+    </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'>
+      <listen type='address' address='127.0.0.1'/>
+    </graphics>
+    <audio id='1' type='none'/>
+    <video>
+      <model type='cirrus' vram='9216' 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>