Synchronise 2025.1 with upstream

nova/tests/fixtures/libvirt.py

@@ -101,6 +101,7 @@ def _reset():
 VIR_DOMAIN_UNDEFINE_MANAGED_SAVE = 1
 VIR_DOMAIN_UNDEFINE_NVRAM = 4
 VIR_DOMAIN_UNDEFINE_KEEP_TPM = 64
+VIR_DOMAIN_UNDEFINE_KEEP_NVRAM = 8
 
 VIR_DOMAIN_AFFECT_CURRENT = 0
 VIR_DOMAIN_AFFECT_LIVE = 1

nova/tests/functional/regressions/test_bug_2125935.py

@@ -0,0 +1,112 @@
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+from unittest import mock
+
+from nova.scheduler import weights
+from nova.tests.functional import integrated_helpers
+
+
+class HostNameWeigher(weights.BaseHostWeigher):
+    # We want to predictabilly have host1 first
+    _weights = {'host1': 1, 'host2': 0}
+
+    def _weigh_object(self, host_state, weight_properties):
+        # Any undefined host gets no weight.
+        return self._weights.get(host_state.host, 0)
+
+
+class TestImagePropsWeigher(integrated_helpers._IntegratedTestBase):
+    """Tests for image props weigher """
+
+    compute_driver = 'fake.MediumFakeDriver'
+    microversion = 'latest'
+    ADMIN_API = True
+
+    def _setup_compute_service(self):
+        # Override to prevent the default 'compute' service from starting
+        pass
+
+    def setUp(self):
+        weight_classes = [
+            __name__ + '.HostNameWeigher',
+            'nova.scheduler.weights.image_props.ImagePropertiesWeigher'
+        ]
+        self.flags(weight_classes=weight_classes,
+                   group='filter_scheduler')
+        self.flags(image_props_weight_multiplier=2.0, group='filter_scheduler')
+        super(TestImagePropsWeigher, self).setUp()
+
+        # Start only the compute services we want for testing
+        self.compute1 = self._start_compute('host1')
+        self.compute2 = self._start_compute('host2')
+
+    @mock.patch('nova.weights.LOG.debug')
+    def test_boot(self, mock_debug):
+        server1 = self._create_server(
+            name='inst1',
+            networks='none',
+            )
+
+        # the weigher sees that there are no existing instances on both hosts
+        # with the same image props
+        mock_debug.assert_any_call(
+            "%s: raw weights %s",
+            "ImagePropertiesWeigher",
+            {('host2', 'host2'): 0.0, ('host1', 'host1'): 0.0})
+        # because of the HostNameWeigher, we're sure that the instance lands
+        # on host1
+        self.assertEqual('host1', server1['OS-EXT-SRV-ATTR:host'])
+        # let's make sure that we don't assert the calls from the previous
+        # schedules.
+        mock_debug.reset_mock()
+
+        server2 = self._create_server(
+            name='inst2',
+            host='host2',
+            networks='none',
+            )
+        # Since we force to a host, the weigher will not be called
+        self.assertEqual('host2', server2['OS-EXT-SRV-ATTR:host'])
+
+        server3 = self._create_server(
+            name='inst3',
+            networks='none',
+            )
+
+        # The weigher is called but it says that there are no existing
+        # instances on both the hosts with the same image props, while we are
+        # sure that the values should be 1.0 for both hosts.
+        # FIXME(sbauza): That's due to bug/2125935
+        mock_debug.assert_any_call(
+            "%s: raw weights %s",
+            "ImagePropertiesWeigher",
+            {('host2', 'host2'): 0.0, ('host1', 'host1'): 0.0})
+        mock_debug.reset_mock()
+        # server3 is now on the same host than host1 as the weigh multiplier
+        # makes the scheduler to pack instances sharing the same image props.
+        self.assertEqual('host1', server3['OS-EXT-SRV-ATTR:host'])
+
+        server4 = self._create_server(
+            name='inst4',
+            networks='none',
+            )
+        # FIXME(sbauza): Same issue. The values should be 2.0 for host1 and 1.0
+        # for host2. That said, due to the fact the HostState is refreshed
+        # already for the previous schedule, the system metadata is existing
+        # for this instance so that's why the weight is 1.0 for host1.
+        mock_debug.assert_any_call(
+            "%s: raw weights %s",
+            "ImagePropertiesWeigher",
+            {('host2', 'host2'): 0.0, ('host1', 'host1'): 1.0})
+        # server4 is now packed with server1 and server3.
+        self.assertEqual('host1', server4['OS-EXT-SRV-ATTR:host'])

nova/tests/unit/virt/libvirt/test_config.py

@@ -2826,6 +2826,7 @@ def _test_config_uefi(self):
         obj.os_mach_type = "pc-q35-5.1"
         obj.os_loader = '/tmp/OVMF_CODE.secboot.fd'
         obj.os_loader_type = 'pflash'
+        obj.os_nvram = '/foo/bar/instance-00000012_VARS.fd'
         obj.os_loader_secure = True
         obj.os_loader_stateless = True
         xml = obj.to_xml()
@@ -2840,6 +2841,7 @@ def _test_config_uefi(self):
               <os>
                 <type machine="pc-q35-5.1">hvm</type>
                 <loader stateless='yes' secure='yes' readonly='yes' type='pflash'>/tmp/OVMF_CODE.secboot.fd</loader>
+                 <nvram>/foo/bar/instance-00000012_VARS.fd</nvram>
               </os>
             </domain>""",  # noqa: E501
             xml,

nova/tests/unit/virt/libvirt/test_driver.py

@@ -18891,10 +18891,11 @@ def test_undefine_domain_disarms_keep_vtpm_if_not_supported(
         fake_guest = mock.Mock()
         mock_get.return_value = fake_guest
 
-        drvr._undefine_domain(instance, keep_vtpm=True)
+        drvr._undefine_domain(instance, keep_vtpm=True, keep_nvram=False)
 
         fake_guest.delete_configuration.assert_called_once_with(
             keep_vtpm=False,
+            keep_nvram=False,
         )
 
         # Check that it truly forces it to False and doesn't do a `not` or
@@ -18904,6 +18905,7 @@ def test_undefine_domain_disarms_keep_vtpm_if_not_supported(
 
         fake_guest.delete_configuration.assert_called_once_with(
             keep_vtpm=False,
+            keep_nvram=False,
         )
 
     @mock.patch.object(host.Host, "get_guest")
@@ -18914,9 +18916,11 @@ def test_undefine_domain_passes_keep_vtpm_if_supported(self, mock_get):
         fake_guest = mock.Mock()
         mock_get.return_value = fake_guest
 
-        drvr._undefine_domain(instance, keep_vtpm=True)
+        drvr._undefine_domain(instance, keep_vtpm=True, keep_nvram=False)
 
-        fake_guest.delete_configuration.assert_called_once_with(keep_vtpm=True)
+        fake_guest.delete_configuration.assert_called_once_with(
+                keep_vtpm=True,
+                keep_nvram=False)
 
         # Check that it does not force keep_vtpm to true, just because it is
         # supported.
@@ -18925,8 +18929,27 @@ def test_undefine_domain_passes_keep_vtpm_if_supported(self, mock_get):
 
         fake_guest.delete_configuration.assert_called_once_with(
             keep_vtpm=False,
+            keep_nvram=False,
         )
 
+    @mock.patch.object(host.Host, "get_guest")
+    def test_undefine_domain_passes_keep_nvram_if_supported(self, mock_get):
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
+        instance = objects.Instance(**self.test_instance)
+        fake_guest = mock.Mock()
+        mock_get.return_value = fake_guest
+        drvr._undefine_domain(instance, keep_nvram=True)
+        fake_guest.delete_configuration.assert_called_once_with(
+            keep_vtpm=False, keep_nvram=True)
+        # Check that it does not force keep_nvram to true, just because it is
+        # supported.
+        fake_guest.reset_mock()
+        drvr._undefine_domain(instance, keep_nvram=False)
+        fake_guest.delete_configuration.assert_called_once_with(
+            keep_vtpm=False,
+            keep_nvram=False,
+            )
+
     @mock.patch.object(host.Host, "list_instance_domains")
     @mock.patch.object(objects.BlockDeviceMappingList, "bdms_by_instance_uuid")
     @mock.patch.object(objects.InstanceList, "get_by_filters")
@@ -19226,7 +19249,7 @@ def test_get_instance_disk_info_from_config_raw_files(self,
         disk_actual_size = 3687091200
         disk_actual_size_blocks = disk_actual_size / 512
         expected_over_committed_disk_size = disk_virtual_size -\
-                                                disk_actual_size
+                disk_actual_size
 
         mock_getsize.return_value = disk_virtual_size
         mock_stat.return_value = mock.Mock(st_blocks=disk_actual_size_blocks)
@@ -21498,7 +21521,10 @@ def test_cleanup_pass(
         mock_delete_files.assert_called_once_with(fake_inst)
         # vTPM secret should not be deleted until instance is deleted.
         mock_delete_vtpm.assert_not_called()
-        mock_undefine.assert_called_once_with(fake_inst, keep_vtpm=False)
+        mock_undefine.assert_called_once_with(
+                fake_inst,
+                keep_vtpm=False,
+                keep_nvram=False)
 
     @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._undefine_domain')
     @mock.patch('nova.crypto.delete_vtpm_secret')
@@ -21524,7 +21550,8 @@ def test_cleanup_preserves_tpm_if_not_destroying_disks(
         mock_get_mapping.assert_called_once_with(None)
         mock_delete_files.assert_not_called()
         mock_delete_vtpm.assert_not_called()
-        mock_undefine.assert_called_once_with(fake_inst, keep_vtpm=True)
+        mock_undefine.assert_called_once_with(fake_inst, keep_vtpm=True,
+                                              keep_nvram=True)
 
     @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._undefine_domain')
     @mock.patch('nova.crypto.delete_vtpm_secret')
@@ -21549,7 +21576,8 @@ def test_cleanup_instance_marked_deleted(
             drvr.cleanup('ctxt', fake_inst, 'netinfo')
         # vTPM secret should not be deleted until instance is deleted.
         mock_delete_vtpm.assert_not_called()
-        mock_undefine.assert_called_once_with(fake_inst, keep_vtpm=False)
+        mock_undefine.assert_called_once_with(fake_inst, keep_vtpm=False,
+                                              keep_nvram=False)
 
     @mock.patch.object(libvirt_driver.LibvirtDriver, 'delete_instance_files',
                        return_value=True)

nova/tests/unit/virt/libvirt/test_guest.py

@@ -145,6 +145,12 @@ def test_delete_configuration_with_keep_vtpm_true(self):
             fakelibvirt.VIR_DOMAIN_UNDEFINE_NVRAM |
             fakelibvirt.VIR_DOMAIN_UNDEFINE_KEEP_TPM)
 
+    def test_delete_configuration_keep_nvram(self):
+        self.guest.delete_configuration(keep_nvram=True)
+        self.domain.undefineFlags.assert_called_once_with(
+            fakelibvirt.VIR_DOMAIN_UNDEFINE_MANAGED_SAVE |
+            fakelibvirt.VIR_DOMAIN_UNDEFINE_KEEP_NVRAM)
+
     def test_delete_configuration_exception(self):
         self.domain.undefineFlags.side_effect = fakelibvirt.libvirtError(
             'oops')

nova/virt/libvirt/driver.py

@@ -1688,7 +1688,7 @@ def destroy(self, context, instance, network_info, block_device_info=None,
         self.cleanup(context, instance, network_info, block_device_info,
                      destroy_disks, destroy_secrets=destroy_secrets)
 
-    def _delete_guest_configuration(self, guest, keep_vtpm):
+    def _delete_guest_configuration(self, guest, keep_vtpm, keep_nvram):
         """Wrapper around guest.delete_configuration which incorporates version
         checks for the additional arguments.
 
@@ -1707,13 +1707,14 @@ def _delete_guest_configuration(self, guest, keep_vtpm):
             )
             keep_vtpm = False
 
-        guest.delete_configuration(keep_vtpm=keep_vtpm)
+        guest.delete_configuration(keep_vtpm=keep_vtpm, keep_nvram=keep_nvram)
 
-    def _undefine_domain(self, instance, keep_vtpm=False):
+    def _undefine_domain(self, instance, keep_vtpm=False, keep_nvram=False):
         try:
             guest = self._host.get_guest(instance)
             try:
-                self._delete_guest_configuration(guest, keep_vtpm=keep_vtpm)
+                self._delete_guest_configuration(guest, keep_vtpm=keep_vtpm,
+                                                 keep_nvram=keep_nvram)
             except libvirt.libvirtError as e:
                 with excutils.save_and_reraise_exception() as ctxt:
                     errcode = e.get_error_code()
@@ -1800,9 +1801,10 @@ def _cleanup(self, context, instance, network_info, block_device_info=None,
         :param destroy_vifs: if plugged vifs should be unplugged
         :param cleanup_instance_dir: If the instance dir should be removed
         :param cleanup_instance_disks: If the instance disks should be removed.
-            Also removes ephemeral encryption secrets, if present.
-        :param destroy_secrets: If the cinder volume encryption libvirt secrets
-                   should be deleted.
+            Also removes ephemeral encryption secrets, if present, as well as
+            vTPM and NVRAM data.
+        :param destroy_secrets: If the cinder volume encryption secrets should
+            be deleted.
         """
         # zero the data on backend pmem device
         vpmems = self._get_vpmems(instance)
@@ -1876,7 +1878,8 @@ def _cleanup(self, context, instance, network_info, block_device_info=None,
                 self._cleanup_ephemeral_encryption_secrets(
                     context, instance, block_device_info)
 
-        self._undefine_domain(instance, keep_vtpm=not cleanup_instance_disks)
+        self._undefine_domain(instance, keep_vtpm=not cleanup_instance_disks,
+                              keep_nvram=not cleanup_instance_disks)
 
     def _cleanup_ephemeral_encryption_secrets(
         self, context, instance, block_device_info
@@ -2450,7 +2453,8 @@ def _swap_volume(self, guest, disk_dev, conf, resize_to):
             # undefine it. If any part of this block fails, the domain is
             # re-defined regardless.
             if guest.has_persistent_configuration():
-                self._delete_guest_configuration(guest, keep_vtpm=True)
+                self._delete_guest_configuration(guest, keep_vtpm=True,
+                                                 keep_nvram=True)
 
             try:
                 dev.copy(conf.to_xml(), reuse_ext=True)
@@ -3579,7 +3583,8 @@ def _live_snapshot(self, context, instance, guest, disk_path, out_path,
             #             If any part of this block fails, the domain is
             #             re-defined regardless.
             if guest.has_persistent_configuration():
-                self._delete_guest_configuration(guest, keep_vtpm=True)
+                self._delete_guest_configuration(guest, keep_vtpm=True,
+                                                 keep_nvram=True)
 
             # NOTE (rmk): Establish a temporary mirror of our root disk and
             #             issue an abort once we have a complete copy.

nova/virt/libvirt/guest.py

@@ -295,12 +295,15 @@ def get_vcpus_info(self):
             yield VCPUInfo(
                 id=vcpu[0], cpu=vcpu[3], state=vcpu[1], time=vcpu[2])
 
-    def delete_configuration(self, keep_vtpm=False):
+    def delete_configuration(self, keep_vtpm=False, keep_nvram=False):
         """Undefines a domain from hypervisor.
 
         :param keep_vtpm: If true, the vTPM data will be preserved. Otherwise,
             it will be deleted. Defaults to false (that is, deleting the vTPM
             data).
+        :param keep_nvram: If true, the NVRAM data will be preserved.
+            Otherwise, it will be deleted. Defaults to false (that is, deleting
+            the NVRAM data).
 
         Calling this with `keep_vtpm` set to True should, eventually, be
         followed up with a call where it is set to False (after re-defining
@@ -312,9 +315,12 @@ def delete_configuration(self, keep_vtpm=False):
         """
         try:
             flags = libvirt.VIR_DOMAIN_UNDEFINE_MANAGED_SAVE
-            flags |= libvirt.VIR_DOMAIN_UNDEFINE_NVRAM
             if keep_vtpm:
                 flags |= libvirt.VIR_DOMAIN_UNDEFINE_KEEP_TPM
+            if keep_nvram:
+                flags |= libvirt.VIR_DOMAIN_UNDEFINE_KEEP_NVRAM
+            else:
+                flags |= libvirt.VIR_DOMAIN_UNDEFINE_NVRAM
             self._domain.undefineFlags(flags)
         except libvirt.libvirtError:
             LOG.debug("Error from libvirt during undefineFlags for guest "

releasenotes/notes/preserve-nvram-ab6d3d2fe923301f.yaml

@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    NVRAM variable store is preserved during stop/start, hard reboot, and live
+    migration by passing the corresponding flag to libvirt.
+
+    See https://bugs.launchpad.net/nova/+bug/1633447 for more details.