Set undefine flags to keep NVRAM (#1329)

Handle enabling and disabling of NVRAM during start domain.

This patch contains latent support for upstream PR fog-libvirt#102 to support destroy
with NVRAM enabled once that is merged.

Fixes #1027.

lib/vagrant-libvirt/action/destroy_domain.rb

@@ -43,11 +43,14 @@ module VagrantPlugins
 
           domain = env[:machine].provider.driver.connection.servers.get(env[:machine].id.to_s)
 
+          undefine_flags = 0
+          undefine_flags |= ProviderLibvirt::Util::DomainFlags::VIR_DOMAIN_UNDEFINE_KEEP_NVRAM if env[:machine].provider_config.nvram
+
           if env[:machine].provider_config.disks.empty? &&
              env[:machine].provider_config.cdroms.empty?
             # if using default configuration of disks and cdroms
             # cdroms are consider volumes, but cannot be destroyed
-            domain.destroy(destroy_volumes: true)
+            destroy_domain(domain, destroy_volumes: true, flags: undefine_flags)
           else
             domain_xml = libvirt_domain.xml_desc(1)
             xml_descr = REXML::Document.new(domain_xml)
@@ -57,7 +60,7 @@ module VagrantPlugins
               env[:ui].warn(I18n.t('vagrant_libvirt.destroy.obsolete_method'))
             end
 
-            domain.destroy(destroy_volumes: false)
+            destroy_domain(domain, destroy_volumes: false, flags: undefine_flags)
 
             volumes = domain.volumes
 
@@ -164,6 +167,14 @@ module VagrantPlugins
             libvirt_disk.destroy if libvirt_disk
           end
         end
+
+        def destroy_domain(domain, destroy_volumes:, flags:)
+          if domain.method(:destroy).parameters.first.include?(:flags)
+            domain.destroy(destroy_volumes: destroy_volumes, flags: flags)
+          else
+            domain.destroy(destroy_volumes: destroy_volumes)
+          end
+        end
       end
     end
   end

lib/vagrant-libvirt/action/start_domain.rb

@@ -375,10 +375,30 @@ module VagrantPlugins
                 end
               end
 
+              undefine_flags = 0
+              nvram = REXML::XPath.first(xml_descr, '/domain/os/nvram')
+              if config.nvram
+                if nvram.nil?
+                  descr_changed = true
+                  nvram = REXML::Element.new('nvram', REXML::XPath.first(xml_descr, '/domain/os'))
+                  nvram.text = config.nvram
+                else
+                  if (nvram.text or '').strip != config.nvram
+                    descr_changed = true
+                    nvram.text = config.nvram
+                  end
+                  undefine_flags |= ProviderLibvirt::Util::DomainFlags::VIR_DOMAIN_UNDEFINE_KEEP_NVRAM
+                end
+              elsif !nvram.nil?
+                descr_changed = true
+                undefine_flags |= ProviderLibvirt::Util::DomainFlags::VIR_DOMAIN_UNDEFINE_NVRAM
+                nvram.parent.delete_element(nvram)
+              end
+
               # Apply
               if descr_changed
                 begin
-                  libvirt_domain.undefine
+                  libvirt_domain.undefine(undefine_flags)
                   new_descr = String.new
                   xml_descr.write new_descr
                   env[:machine].provider.driver.connection.servers.create(xml: new_descr)

lib/vagrant-libvirt/util.rb

@@ -9,6 +9,7 @@ module VagrantPlugins
       autoload :NetworkUtil, 'vagrant-libvirt/util/network_util'
       autoload :StorageUtil, 'vagrant-libvirt/util/storage_util'
       autoload :ErrorCodes, 'vagrant-libvirt/util/error_codes'
+      autoload :DomainFlags, 'vagrant-libvirt/util/domain_flags'
       autoload :Ui, 'vagrant-libvirt/util/ui'
     end
   end

lib/vagrant-libvirt/util/domain_flags.rb

@@ -0,0 +1,15 @@
+ # Ripped from https://libvirt.org/html/libvirt-libvirt-domain.html#types
+module VagrantPlugins
+  module ProviderLibvirt
+    module Util
+      module DomainFlags
+        # virDomainUndefineFlagsValues
+        VIR_DOMAIN_UNDEFINE_MANAGED_SAVE = 1 # Also remove any managed save
+        VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA = 2 # If last use of domain, then also remove any snapshot metadata
+        VIR_DOMAIN_UNDEFINE_NVRAM = 4 # Also remove any nvram file
+        VIR_DOMAIN_UNDEFINE_KEEP_NVRAM = 8 # Keep nvram file
+        VIR_DOMAIN_UNDEFINE_CHECKPOINTS_METADATA = 16 # If last use of domain, then also remove any checkpoint metadata Future undefine control flags should come here.
+      end
+    end
+  end
+end

spec/unit/action/destroy_domain_spec.rb

@@ -19,10 +19,14 @@ describe VagrantPlugins::ProviderLibvirt::Action::DestroyDomain do
 
   let(:domain_xml) { File.read(File.join(File.dirname(__FILE__), File.basename(__FILE__, '.rb'), domain_xml_file)) }
 
+  let(:destroy_method) { double('destroy_method') }
+
   before do
     allow(machine.provider).to receive('driver').and_return(driver)
     allow(driver).to receive(:connection).and_return(connection)
     allow(logger).to receive(:info)
+    allow(domain).to receive(:method).with(:destroy).and_return(destroy_method)
+    allow(destroy_method).to receive(:parameters).and_return([[:opt, :options, :flags]])
   end
 
   describe '#call' do
@@ -49,7 +53,7 @@ describe VagrantPlugins::ProviderLibvirt::Action::DestroyDomain do
 
       context 'when box only has one root disk' do
         it 'calls fog to destroy volumes' do
-          expect(domain).to receive(:destroy).with(destroy_volumes: true)
+          expect(domain).to receive(:destroy).with(destroy_volumes: true, flags: 0)
           expect(subject.call(env)).to be_nil
         end
 
@@ -69,7 +73,7 @@ describe VagrantPlugins::ProviderLibvirt::Action::DestroyDomain do
           end
 
           it 'destroys disks individually' do
-            expect(domain).to receive(:destroy).with(destroy_volumes: false)
+            expect(domain).to receive(:destroy).with(destroy_volumes: false, flags: 0)
             expect(extra_disk).to receive(:destroy) # extra disk remove
             expect(root_disk).to receive(:destroy)  # root disk remove
             expect(subject.call(env)).to be_nil
@@ -81,7 +85,7 @@ describe VagrantPlugins::ProviderLibvirt::Action::DestroyDomain do
         let(:domain_xml_file) { 'box_multiple_disks.xml' }
 
         it 'calls fog to destroy volumes' do
-          expect(domain).to receive(:destroy).with(destroy_volumes: true)
+          expect(domain).to receive(:destroy).with(destroy_volumes: true, flags: 0)
           expect(subject.call(env)).to be_nil
         end
 
@@ -111,7 +115,7 @@ describe VagrantPlugins::ProviderLibvirt::Action::DestroyDomain do
               expect(disk).to receive(:name).and_return(name).at_least(:once)
               expect(disk).to receive(:destroy)
             end
-            expect(domain).to receive(:destroy).with(destroy_volumes: false)
+            expect(domain).to receive(:destroy).with(destroy_volumes: false, flags: 0)
             expect(subject.call(env)).to be_nil
           end
 
@@ -133,7 +137,7 @@ describe VagrantPlugins::ProviderLibvirt::Action::DestroyDomain do
                 next if disk == domain_disks.last.first
                 expect(disk).to receive(:destroy)
               end
-              expect(domain).to receive(:destroy).with(destroy_volumes: false)
+              expect(domain).to receive(:destroy).with(destroy_volumes: false, flags: 0)
               expect(subject.call(env)).to be_nil
             end
 
@@ -150,7 +154,7 @@ describe VagrantPlugins::ProviderLibvirt::Action::DestroyDomain do
                   next if domain_disks.last.first == disk
                   expect(disk).to receive(:destroy)
                 end
-                expect(domain).to receive(:destroy).with(destroy_volumes: false)
+                expect(domain).to receive(:destroy).with(destroy_volumes: false, flags: 0)
                 expect(subject.call(env)).to be_nil
               end
 
@@ -176,7 +180,7 @@ describe VagrantPlugins::ProviderLibvirt::Action::DestroyDomain do
                     next if domain_disks.last.first == disk
                     expect(disk).to receive(:destroy)
                   end
-                  expect(domain).to receive(:destroy).with(destroy_volumes: false)
+                  expect(domain).to receive(:destroy).with(destroy_volumes: false, flags: 0)
                   expect(subject.call(env)).to be_nil
                 end
               end
@@ -185,6 +189,35 @@ describe VagrantPlugins::ProviderLibvirt::Action::DestroyDomain do
         end
       end
 
+      context 'when has nvram' do
+        let(:vagrantfile) do
+          <<-EOF
+          Vagrant.configure('2') do |config|
+            config.vm.define :test
+            config.vm.provider :libvirt do |libvirt|
+              libvirt.nvram = "test"
+            end
+          end
+          EOF
+        end
+
+        it 'sets destroy flags to keep nvram' do
+          expect(domain).to receive(:destroy).with(destroy_volumes: true, flags: VagrantPlugins::ProviderLibvirt::Util::DomainFlags::VIR_DOMAIN_UNDEFINE_KEEP_NVRAM)
+          expect(subject.call(env)).to be_nil
+        end
+
+        context 'when fog does not support destroy with flags' do
+          before do
+            expect(destroy_method).to receive(:parameters).and_return([[:opt, :options]])
+          end
+
+          it 'skips setting additional destroy flags' do
+            expect(domain).to receive(:destroy).with(destroy_volumes: true)
+            expect(subject.call(env)).to be_nil
+          end
+        end
+      end
+
       context 'when has CDROMs attached' do
         let(:vagrantfile_providerconfig) do
           <<-EOF
@@ -197,7 +230,7 @@ describe VagrantPlugins::ProviderLibvirt::Action::DestroyDomain do
           expect(domain).to receive(:volumes).and_return([root_disk, nil])
           expect(libvirt_domain).to receive(:xml_desc).and_return(domain_xml)
 
-          expect(domain).to_not receive(:destroy).with(destroy_volumes: true)
+          expect(domain).to_not receive(:destroy).with(destroy_volumes: true, flags: 0)
           expect(root_disk).to receive(:destroy)  # root disk remove
           expect(subject.call(env)).to be_nil
         end

spec/unit/action/start_domain_spec.rb

@@ -62,6 +62,62 @@ describe VagrantPlugins::ProviderLibvirt::Action::StartDomain do
       end
     end
 
+    context 'nvram' do
+      context 'when being added to existing' do
+        let(:vagrantfile_providerconfig) do
+          <<-EOF
+          libvirt.nvram = "/path/to/nvram/file"
+          EOF
+        end
+        let(:test_file) { 'existing.xml' }
+        let(:updated_test_file) { 'existing_added_nvram.xml' }
+
+        it 'should undefine without passing flags' do
+          expect(libvirt_domain).to receive(:undefine).with(0)
+          expect(servers).to receive(:create).with(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 'when it was already in use' do
+        let(:vagrantfile_providerconfig) do
+          <<-EOF
+          libvirt.nvram = "/path/to/nvram/file"
+          # change another setting to trigger the undefine/create
+          libvirt.cpus = 4
+          EOF
+        end
+        let(:test_file) { 'nvram_domain.xml' }
+        let(:updated_test_file) { 'nvram_domain_other_setting.xml' }
+
+        it 'should set the flag to keep nvram' do
+          expect(libvirt_domain).to receive(:undefine).with(VagrantPlugins::ProviderLibvirt::Util::DomainFlags::VIR_DOMAIN_UNDEFINE_KEEP_NVRAM)
+          expect(servers).to receive(:create).with(xml: updated_domain_xml)
+          expect(libvirt_domain).to receive(:autostart=)
+          expect(domain).to receive(:start)
+
+          expect(subject.call(env)).to be_nil
+        end
+
+        context 'when it is being disabled' do
+          let(:vagrantfile_providerconfig) { }
+          let(:updated_test_file) { 'nvram_domain_removed.xml' }
+
+          it 'should set the flag to remove nvram' do
+            expect(libvirt_domain).to receive(:undefine).with(VagrantPlugins::ProviderLibvirt::Util::DomainFlags::VIR_DOMAIN_UNDEFINE_NVRAM)
+            expect(servers).to receive(:create).with(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 'tpm' do
       context 'passthrough tpm added' do
         let(:updated_test_file) { 'default_added_tpm_path.xml' }

spec/unit/action/start_domain_spec/existing_added_nvram.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'/>
+  <nvram>/path/to/nvram/file</nvram></os>
+  <features>
+    <acpi/>
+    <apic/>
+    <pae/>
+  </features>
+  <cpu check='partial' mode='host-model'/>
+  <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='piix3-uhci' type='usb'>
+      <address bus='0x00' domain='0x0000' function='0x2' slot='0x01' type='pci'/>
+    </controller>
+    <controller index='0' model='pci-root' type='pci'/>
+    <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>

spec/unit/action/start_domain_spec/nvram_domain.xml

@@ -0,0 +1,63 @@
+<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'/>
+    <nvram>/path/to/nvram/file</nvram>
+  </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='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>

spec/unit/action/start_domain_spec/nvram_domain_other_setting.xml

@@ -0,0 +1,63 @@
+<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'>4</vcpu>
+  <os>
+    <type arch='x86_64' machine='pc-i440fx-6.0'>hvm</type>
+    <boot dev='hd'/>
+    <nvram>/path/to/nvram/file</nvram>
+  </os>
+  <features>
+    <acpi/>
+    <apic/>
+    <pae/>
+  </features>
+  <cpu check='partial' mode='host-model'/>
+  <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='piix3-uhci' type='usb'>
+      <address bus='0x00' domain='0x0000' function='0x2' slot='0x01' type='pci'/>
+    </controller>
+    <controller index='0' model='pci-root' type='pci'/>
+    <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>

spec/unit/action/start_domain_spec/nvram_domain_removed.xml

@@ -0,0 +1,63 @@
+<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-model'/>
+  <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='piix3-uhci' type='usb'>
+      <address bus='0x00' domain='0x0000' function='0x2' slot='0x01' type='pci'/>
+    </controller>
+    <controller index='0' model='pci-root' type='pci'/>
+    <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>