diff --git a/doc/source/admin/scheduling.rst b/doc/source/admin/scheduling.rst index ea68cc1b72e..7b79c503065 100644 --- a/doc/source/admin/scheduling.rst +++ b/doc/source/admin/scheduling.rst @@ -1104,6 +1104,14 @@ provided in the option value. The resulted host weight would then be multiplied by the value of :oslo.config:option:`filter_scheduler.image_props_weight_multiplier`. + +.. note:: + + The weigher compares the values of the image properties as strings. If some + image properties are lists (eg. hw_numa_nodes), then if the values are + ordered differently, then the weigher will consider them as different + values. + Utilization-aware scheduling ---------------------------- diff --git a/nova/api/openstack/compute/servers.py b/nova/api/openstack/compute/servers.py index 5c1617e7fb8..027b2a7764b 100644 --- a/nova/api/openstack/compute/servers.py +++ b/nova/api/openstack/compute/servers.py @@ -512,12 +512,11 @@ def _process_bdms_for_create( create_kwargs['legacy_bdm'] = True elif block_device_mapping_v2: # Have to check whether --image is given, see bug 1433609 - image_href = server_dict.get('imageRef') - image_uuid_specified = image_href is not None + image_ref = server_dict.get('imageRef') try: block_device_mapping = [ block_device.BlockDeviceDict.from_api(bdm_dict, - image_uuid_specified) + image_uuid_specified=image_ref not in {None, ''}) for bdm_dict in block_device_mapping_v2] except exception.InvalidBDMFormat as e: raise exc.HTTPBadRequest(explanation=e.format_message()) diff --git a/nova/objects/instance.py b/nova/objects/instance.py index cf72d76d68e..181686e9aa5 100644 --- a/nova/objects/instance.py +++ b/nova/objects/instance.py @@ -1606,7 +1606,8 @@ def fill_metadata(self): for inst in [i for i in self if i.uuid in updated]: # Patch up our instances with system_metadata from the fill # operation - inst.system_metadata = utils.instance_sys_meta(updated) + inst.system_metadata = utils.instance_sys_meta( + updated[inst.uuid]) @base.remotable_classmethod def get_uuids_by_host(cls, context, host): diff --git a/nova/scheduler/weights/image_props.py b/nova/scheduler/weights/image_props.py index c28e908576c..12284080b0a 100644 --- a/nova/scheduler/weights/image_props.py +++ b/nova/scheduler/weights/image_props.py @@ -74,10 +74,18 @@ def _weigh_object(self, host_state, request_spec): # context so we can access all of them, not only the ones from the # request. ctxt = nova_context.get_admin_context() - insts = objects.InstanceList(ctxt, - objects=host_state.instances.values()) # system_metadata isn't loaded yet, let's do this. - insts.fill_metadata() + # Given all instances are in the same host, we can target the same + # cell. + try: + cell_mapping = objects.CellMapping.get_by_uuid( + ctxt, host_state.cell_uuid) + except exception.CellMappingNotFound: + return weight + with nova_context.target_cell(ctxt, cell_mapping) as cell_ctxt: + insts = objects.InstanceList(cell_ctxt, + objects=host_state.instances.values()) + insts.fill_metadata() for inst in insts: try: diff --git a/nova/tests/fixtures/libvirt.py b/nova/tests/fixtures/libvirt.py index 423c1418933..c939e021c48 100644 --- a/nova/tests/fixtures/libvirt.py +++ b/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 diff --git a/nova/tests/functional/regressions/test_bug_2077980.py b/nova/tests/functional/regressions/test_bug_2077980.py new file mode 100644 index 00000000000..05b27ede7f7 --- /dev/null +++ b/nova/tests/functional/regressions/test_bug_2077980.py @@ -0,0 +1,49 @@ +# 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 oslo_utils.fixture import uuidsentinel as uuids + +from nova.tests.functional.api import client +from nova.tests.functional import integrated_helpers + + +class TestImageToLocalBDM(integrated_helpers._IntegratedTestBase): + """Regression test for bug 2077980 + + This regression test asserts the behaviour of server creation requests when + using a BDM with source_type=image, destination_type=local, and + boot_index=0, which should result in a failure. + """ + microversion = 'latest' + + # TODO(stephenfin): We should eventually fail regardless of the value of + # imageRef, but that requires an API microversion + def test_image_to_local_bdm__empty_image(self): + """Assert behaviour when booting with imageRef set to empty string""" + server = { + 'name': 'test_image_to_local_bdm__null_image', + 'imageRef': '', + 'flavorRef': '1', # m1.tiny from DefaultFlavorsFixture, + 'networks': 'none', + 'block_device_mapping_v2': [{ + 'boot_index': 0, + 'uuid': uuids.image, + 'source_type': 'image', + 'destination_type': 'local'}], + } + # NOTE(stephenfin): This should always fail as we don't allow users to + # set this as it would conflict with the flavor definition. + ex = self.assertRaises( + client.OpenStackApiException, self.api.post_server, + {'server': server}) + self.assertEqual(400, ex.response.status_code) + self.assertIn("Mapping image to local is not supported.", str(ex)) diff --git a/nova/tests/functional/regressions/test_bug_2125935.py b/nova/tests/functional/regressions/test_bug_2125935.py new file mode 100644 index 00000000000..45810c0e23e --- /dev/null +++ b/nova/tests/functional/regressions/test_bug_2125935.py @@ -0,0 +1,107 @@ +# 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', + ) + + # now the weigher sees that host1 has an instance with the same image + # props + mock_debug.assert_any_call( + "%s: raw weights %s", + "ImagePropertiesWeigher", + {('host2', 'host2'): 1.0, ('host1', 'host1'): 1.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', + ) + # eventually, the weigher sees the two existing instances on host1 + mock_debug.assert_any_call( + "%s: raw weights %s", + "ImagePropertiesWeigher", + {('host2', 'host2'): 1.0, ('host1', 'host1'): 2.0}) + # server4 is now packed with server1 and server3. + self.assertEqual('host1', server4['OS-EXT-SRV-ATTR:host']) diff --git a/nova/tests/unit/objects/test_instance.py b/nova/tests/unit/objects/test_instance.py index 0e041ecd377..b97964c222d 100644 --- a/nova/tests/unit/objects/test_instance.py +++ b/nova/tests/unit/objects/test_instance.py @@ -2066,6 +2066,8 @@ def test_fill_metadata(self): 'host': 'foo'} for i in range(5): + # persist some metadata directly in the DB + values['system_metadata'] = {'BTTF2': '2015'} db.instance_create(self.context, dict(values)) @@ -2089,12 +2091,17 @@ def test_fill_metadata(self): self.assertEqual(0, len([i for i in insts if 'system_metadata' not in i])) + #  Inst 1 is now updated with the new metadata + self.assertEqual({'BTTF1': '1955'}, insts[1].system_metadata) + # Inst 2 should have not had its in-memory copy clobbered self.assertEqual({'bttf3': '1885'}, insts[2].system_metadata) - # Inst 1 should have system_metadata loaded, but empty - self.assertIn('system_metadata', insts[0]) - self.assertEqual({}, insts[0].system_metadata) + # Other instances should have system_metadata loaded directly from the + # DB + for i in [0, 3, 4]: + self.assertIn('system_metadata', insts[i]) + self.assertEqual({'BTTF2': '2015'}, insts[i].system_metadata) def test_fill_metadata_nop(self): insts = objects.InstanceList([objects.Instance(uuid=uuids.inst, diff --git a/nova/tests/unit/scheduler/weights/test_weights_image_props.py b/nova/tests/unit/scheduler/weights/test_weights_image_props.py index c32ef96dafb..2610a6569f7 100644 --- a/nova/tests/unit/scheduler/weights/test_weights_image_props.py +++ b/nova/tests/unit/scheduler/weights/test_weights_image_props.py @@ -16,6 +16,8 @@ from oslo_utils.fixture import uuidsentinel as uuids +from nova import context as nova_context +from nova import exception from nova import objects from nova.scheduler import weights from nova.scheduler.weights import image_props @@ -36,7 +38,7 @@ properties=objects.ImageMetaProps(os_distro='linux', hw_machine_type='pc')) -class ImagePropertiesWeigherTestCase(test.NoDBTestCase): +class _ImagePropertiesWeigherBase(test.NoDBTestCase): def setUp(self): super().setUp() @@ -90,6 +92,9 @@ def _get_all_hosts(self): return [fakes.FakeHostState(host, node, values) for host, node, values in host_values] + +class ImagePropertiesWeigherTestCase(_ImagePropertiesWeigherBase): + @mock.patch('nova.objects.InstanceList.fill_metadata') def test_multiplier_default(self, mock_fm): hostinfo_list = self._get_all_hosts() @@ -194,3 +199,71 @@ def test_multiplier_per_property(self, mock_fm): self.assertEqual('host3', weights[0].obj.host) mock_fm.assert_has_calls([mock.call(), mock.call(), mock.call(), mock.call()]) + + +class TestTargetCellCalled(_ImagePropertiesWeigherBase): + # Using real cell infrastructure instead of SingleCellSimple fixture + # as we need to verify set_target_cell calls are made + USES_DB_SELF = True + + @mock.patch('nova.objects.InstanceList.fill_metadata') + @mock.patch.object(nova_context, 'set_target_cell') + @mock.patch('nova.objects.CellMapping.get_by_uuid') + @mock.patch.object(nova_context.RequestContext, 'from_dict') + @mock.patch.object(nova_context, 'get_admin_context') + def test_target_cell_called(self, mock_get_admin_context, mock_from_dict, + mock_get_by_uuid, mock_target_cell, mock_fm): + fake_context = nova_context.RequestContext(is_admin=True) + fake_cell_ctx = nova_context.RequestContext(is_admin=True) + # let's simulate that we set a cell context as target_cell calls + # from_dict internally + mock_from_dict.return_value = fake_cell_ctx + mock_get_admin_context.return_value = fake_context + self.flags(image_props_weight_multiplier=1.0, group='filter_scheduler') + + fake_cell_mapping = objects.CellMapping(uuid=uuids.cell1) + mock_get_by_uuid.side_effect = [fake_cell_mapping, fake_cell_mapping, + # host3 won't have a cell mapping + exception.CellMappingNotFound( + uuid=uuids.cell2)] + + # Just create three hosts with only one instance, we just want to test + # the calls to target_cell and fill_metadata. + hostinfo_list = [ + fakes.FakeHostState('host1', 'node1', + {'instances': {uuids.inst1: objects.Instance( + uuid=uuids.inst1)}, + 'cell_uuid': uuids.cell1}), + fakes.FakeHostState('host2', 'node2', + {'instances': {}, 'cell_uuid': uuids.cell1}), + # host3 is in a different cell + fakes.FakeHostState('host3', 'node3', + {'instances': {}, 'cell_uuid': uuids.cell2}), + ] + request_spec = objects.RequestSpec(image=PROP_WIN_PC) + + weights = self.weight_handler.get_weighed_objects( + self.weighers, hostinfo_list, weighing_properties=request_spec) + + mock_get_by_uuid.assert_has_calls([ + mock.call(fake_context, uuids.cell1), + mock.call(fake_context, uuids.cell1), + mock.call(fake_context, uuids.cell2)]) + + # Now we see that set_target_cell is called with the cell context only + # twice given host3 does not have a cell mapping + mock_target_cell.assert_has_calls( + [mock.call(fake_cell_ctx, fake_cell_mapping), + mock.call(fake_cell_ctx, fake_cell_mapping)]) + + # Ensure that fill_metadata was called with the correct context + def _fake_fill_metadata(_self): + self.assertEqual(fake_cell_ctx, _self._context) + mock_fm.side_effect = _fake_fill_metadata + + # Double check that we called it only twice + self.assertEqual(2, mock_fm.call_count) + + # host1 wins but we return all three hosts. + self.assertEqual(3, len(weights)) + self.assertEqual('host1', weights[0].obj.host) diff --git a/nova/tests/unit/virt/libvirt/test_config.py b/nova/tests/unit/virt/libvirt/test_config.py index 8ae3f9c723d..4bf8ca17373 100644 --- a/nova/tests/unit/virt/libvirt/test_config.py +++ b/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): hvm /tmp/OVMF_CODE.secboot.fd + /foo/bar/instance-00000012_VARS.fd """, # noqa: E501 xml, diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 064c67f21c4..a74602984b7 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/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) diff --git a/nova/tests/unit/virt/libvirt/test_guest.py b/nova/tests/unit/virt/libvirt/test_guest.py index 359013c54ea..3034ff0a9d2 100644 --- a/nova/tests/unit/virt/libvirt/test_guest.py +++ b/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') diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 7054b0a1355..361522e8e30 100644 --- a/nova/virt/libvirt/driver.py +++ b/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. diff --git a/nova/virt/libvirt/guest.py b/nova/virt/libvirt/guest.py index 6d7eb969df9..065181d89e6 100644 --- a/nova/virt/libvirt/guest.py +++ b/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 " diff --git a/releasenotes/notes/bug-2125935-de97147d5ff960b5.yaml b/releasenotes/notes/bug-2125935-de97147d5ff960b5.yaml new file mode 100644 index 00000000000..e07482f5313 --- /dev/null +++ b/releasenotes/notes/bug-2125935-de97147d5ff960b5.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + Previously the ImagePropertiesWeigher was not running correctly as it wasn't + targeting the correct cell context for getting the system metadata of the + instances. This was fixed in bug `#2125935`_. + + .. _`#2125935`: https://bugs.launchpad.net/nova/+bug/2125935 diff --git a/releasenotes/notes/preserve-nvram-ab6d3d2fe923301f.yaml b/releasenotes/notes/preserve-nvram-ab6d3d2fe923301f.yaml new file mode 100644 index 00000000000..1dc0a594e9a --- /dev/null +++ b/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.