From e6f8bf5ff5c0a7ae811e4635e947d58e60d13c06 Mon Sep 17 00:00:00 2001 From: songjie Date: Fri, 5 Jan 2024 17:25:58 +0800 Subject: [PATCH] hardware: Fix image_meta.id within get_mem_encryption_constraint In get_mem_encryption_constraint, image_meta.id may not exist. For example, a volume booted instance. So we check whether the id exists. If there is no value, we set it to a sentinel value that can be detected later. i.e. '', as Sean suggested. Closes-Bug: #2041511 Related-Bug: #2047399 Change-Id: Id4a9d3972589ed7f1c21ffbe0a13d61136b73470 Signed-off-by: songjie --- nova/tests/unit/virt/test_hardware.py | 63 +++++++++++++++++++++++++++ nova/virt/hardware.py | 11 ++++- 2 files changed, 72 insertions(+), 2 deletions(-) diff --git a/nova/tests/unit/virt/test_hardware.py b/nova/tests/unit/virt/test_hardware.py index ffecf4e38e97..d0a96fd07e70 100644 --- a/nova/tests/unit/virt/test_hardware.py +++ b/nova/tests/unit/virt/test_hardware.py @@ -5454,6 +5454,16 @@ class MemEncryptionRequestedWithInvalidMachineTypeTestCase( enc_image_prop, image_props, error_data) + def _test_encrypted_memory_support_pc_image_id(self, image_meta, + error_data): + extra_specs = {'hw:mem_encryption': True} + flavor = objects.Flavor(name=self.flavor_name, + extra_specs=extra_specs) + exc = self.assertRaises(self.expected_exception, + hw.get_mem_encryption_constraint, + flavor, image_meta) + self.assertEqual(self.expected_error % error_data, str(exc)) + def test_flavor_requires_encrypted_memory_support_pc(self): for extra_spec in ('1', 'true', 'True'): self._test_encrypted_memory_support_pc(extra_spec, None) @@ -5468,6 +5478,31 @@ class MemEncryptionRequestedWithInvalidMachineTypeTestCase( self._test_encrypted_memory_support_pc( extra_spec, image_prop) + def test_encrypted_memory_support_pc_no_image_id(self): + image_props = {'hw_mem_encryption': True, + 'hw_firmware_type': 'uefi', + 'hw_machine_type': 'pc'} + image_meta = fake_image_obj( + {'name': self.image_name}, {}, image_props) + error_data = {'image_id': '', + 'image_name': self.image_name, + 'mtype': 'pc'} + self._test_encrypted_memory_support_pc_image_id(image_meta, + error_data) + + def test_encrypted_memory_support_pc_has_image_id(self): + image_props = {'hw_mem_encryption': True, + 'hw_firmware_type': 'uefi', + 'hw_machine_type': 'pc'} + image_meta = fake_image_obj( + {'id': self.image_id, 'name': self.image_name}, + {}, image_props) + error_data = {'image_id': self.image_id, + 'image_name': self.image_name, + 'mtype': 'pc'} + self._test_encrypted_memory_support_pc_image_id(image_meta, + error_data) + class MemEncryptionRequiredTestCase(test.NoDBTestCase): flavor_name = "m1.faketiny" @@ -5544,6 +5579,34 @@ class MemEncryptionRequiredTestCase(test.NoDBTestCase): hw.get_mem_encryption_constraint, flavor, image_meta) + @mock.patch.object(hw, 'LOG') + def test_encrypted_memory_support_no_id_for_volume(self, mock_log): + extra_specs = {'hw:mem_encryption': True} + flavor = objects.Flavor(name=self.flavor_name, + extra_specs=extra_specs) + + image_props = {'hw_mem_encryption': True, + 'hw_firmware_type': 'uefi'} + image_meta = objects.ImageMeta.from_dict({ + 'min_disk': 0, + 'min_ram': 0, + 'properties': image_props, + 'size': 0, + 'status': 'active'}) + + self.assertTrue(hw.get_mem_encryption_constraint(flavor, + image_meta)) + + requesters = "hw:mem_encryption extra spec in %s flavor and " \ + "hw_mem_encryption property of image %s" % \ + (self.flavor_name, '') + # Confirm that it can handle the situation that there is + # no image_id when booting from volume. We set it to + # '' that we can detect later. + mock_log.debug.assert_has_calls([ + mock.call("Memory encryption requested by %s", requesters) + ]) + class PCINUMAAffinityPolicyTest(test.NoDBTestCase): diff --git a/nova/virt/hardware.py b/nova/virt/hardware.py index 00c1b4af8bee..53a690bc5669 100644 --- a/nova/virt/hardware.py +++ b/nova/virt/hardware.py @@ -1191,13 +1191,18 @@ def get_mem_encryption_constraint( # If we get this far, either the extra spec or image property explicitly # specified a requirement regarding memory encryption, and if both did, # they are asking for the same thing. + # NOTE(jie, sean): When creating a volume booted instance with memory + # encryption enabled, image_meta has no id key. See bug #2041511. + # So we check whether id exists. If there is no value, we set it + # to a sentinel value we can detect later. i.e. ''. requesters = [] if flavor_mem_enc: requesters.append("hw:mem_encryption extra spec in %s flavor" % flavor.name) if image_mem_enc: + image_id = (image_meta.id if 'id' in image_meta else '') requesters.append("hw_mem_encryption property of image %s" % - image_meta.id) + image_id) _check_mem_encryption_uses_uefi_image(requesters, image_meta) _check_mem_encryption_machine_type(image_meta, machine_type) @@ -1275,12 +1280,14 @@ def _check_mem_encryption_machine_type(image_meta, machine_type=None): # image_meta.name is not set if image object represents root Cinder volume. image_name = (image_meta.name if 'name' in image_meta else None) + # image_meta.id is not set when booting from volume. + image_id = (image_meta.id if 'id' in image_meta else '') # Could be something like pc-q35-2.11 if a specific version of the # machine type is required, so do substring matching. if 'q35' not in mach_type: raise exception.InvalidMachineType( mtype=mach_type, - image_id=image_meta.id, image_name=image_name, + image_id=image_id, image_name=image_name, reason=_("q35 type is required for SEV to work"))