openstack
/
nova
Code Issues Proposed changes

Always delete NVRAM files when deleting instances 

When deleting an instance, always send VIR_DOMAIN_UNDEFINE_NVRAM to
delete the NVRAM file, regardless of whether the image is of type UEFI.
This prevents a bug when rebuilding an instance from an UEFI image to a
non-UEFI image.

Closes-Bug: #1997352

Change-Id: I24648f5b7895bf5d093f222b6c6e364becbb531f
Signed-off-by: Simon Hensel <simon.hensel@inovex.de>
This commit is contained in:
Simon Hensel 2024-01-23 16:16:17 +01:00
parent 6531ed6310
commit 406d590a36
4 changed files with 17 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
nova/tests/unit/virt/libvirt/test_driver.py
View File

@ -5406,13 +5406,6 @@ class LibvirtConnTestCase(test.NoDBTestCase,
'/usr/share/OVMF/OVMF_VARS.fd', cfg.os_nvram_template)
self.assertFalse(cfg.os_loader_secure)
def test_check_uefi_support_aarch64(self):
self.mock_uname.return_value = fakelibvirt.os_uname(
'Linux', '', '5.4.0-0-generic', '', fields.Architecture.AARCH64)
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
image_meta = objects.ImageMeta.from_dict(self.test_image_meta)
self.assertTrue(drvr._check_uefi_support(image_meta))
def test_get_guest_config_with_block_device(self):
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
@ -17645,7 +17638,7 @@ class LibvirtConnTestCase(test.NoDBTestCase,
self.assertEqual(2, mock_domain.ID.call_count)
mock_domain.destroy.assert_called_once_with()
mock_domain.undefineFlags.assert_called_once_with(1)
mock_domain.undefineFlags.assert_called_once_with(5)
mock_domain.undefine.assert_called_once_with()
mock_save.assert_called_once_with()
@ -17667,7 +17660,7 @@ class LibvirtConnTestCase(test.NoDBTestCase,
self.assertEqual(1, mock_domain.ID.call_count)
mock_domain.destroy.assert_called_once_with()
mock_domain.undefineFlags.assert_called_once_with(1)
mock_domain.undefineFlags.assert_called_once_with(5)
mock_domain.hasManagedSaveImage.assert_has_calls([mock.call(0)])
mock_domain.managedSaveRemove.assert_called_once_with(0)
mock_domain.undefine.assert_called_once_with()
@ -17692,7 +17685,7 @@ class LibvirtConnTestCase(test.NoDBTestCase,
self.assertEqual(1, mock_domain.ID.call_count)
mock_domain.destroy.assert_called_once_with()
mock_domain.undefineFlags.assert_called_once_with(1)
mock_domain.undefineFlags.assert_called_once_with(5)
mock_domain.hasManagedSaveImage.assert_has_calls([mock.call(0)])
mock_domain.undefine.assert_called_once_with()
mock_save.assert_called_once_with()
@ -17718,10 +17711,10 @@ class LibvirtConnTestCase(test.NoDBTestCase,
self.assertEqual(1, mock_domain.ID.call_count)
mock_domain.destroy.assert_called_once_with()
# NVRAM flag should not called only host support uefi
mock_domain.undefineFlags.assert_called_once_with(
fakelibvirt.VIR_DOMAIN_UNDEFINE_MANAGED_SAVE
)
mock_domain.undefineFlags.assert_has_calls([mock.call(
fakelibvirt.VIR_DOMAIN_UNDEFINE_MANAGED_SAVE |
fakelibvirt.VIR_DOMAIN_UNDEFINE_NVRAM
)])
mock_domain.undefine.assert_not_called()
mock_save.assert_called_once_with()
@ -17746,8 +17739,6 @@ class LibvirtConnTestCase(test.NoDBTestCase,
self.assertEqual(1, mock_domain.ID.call_count)
mock_domain.destroy.assert_called_once_with()
# undefineFlags should now be called with 5 as uefi supported
# by both host and guest
mock_domain.undefineFlags.assert_has_calls([mock.call(
fakelibvirt.VIR_DOMAIN_UNDEFINE_MANAGED_SAVE |
fakelibvirt.VIR_DOMAIN_UNDEFINE_NVRAM
@ -20676,7 +20667,7 @@ class LibvirtConnTestCase(test.NoDBTestCase,
resize_to = 1
expected_resize_to = resize_to * units.Gi
drvr._swap_volume(mock_guest, 'vdb', mock_conf, resize_to, None)
drvr._swap_volume(mock_guest, 'vdb', mock_conf, resize_to)
# Assert that virDomainBlockCopy is called
mock_dev.copy.assert_called_once_with(
@ -20716,7 +20707,7 @@ class LibvirtConnTestCase(test.NoDBTestCase,
# Assert that exception.VolumeRebaseFailed is raised
self.assertRaises(exception.VolumeRebaseFailed, drvr._swap_volume,
mock_guest, 'vdb', mock_conf, 0, None)
mock_guest, 'vdb', mock_conf, 0)
# Assert that virDomainBlockCopy is called
mock_dev.copy.assert_called_once_with(
@ -20735,9 +20726,6 @@ class LibvirtConnTestCase(test.NoDBTestCase,
swap_volume, disconnect_volume):
conn = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI())
instance = objects.Instance(**self.test_instance)
image_meta = objects.ImageMeta.from_dict(self.test_image_meta)
hw_firmware_type = image_meta.properties.get(
'hw_firmware_type')
old_connection_info = {'driver_volume_type': 'fake',
'serial': 'old-volume-id',
'data': {'device_path': '/fake-old-volume',
@ -20770,8 +20758,7 @@ class LibvirtConnTestCase(test.NoDBTestCase,
connect_volume.assert_called_once_with(self.context,
new_connection_info, instance)
swap_volume.assert_called_once_with(guest, 'vdb',
conf, 1, hw_firmware_type)
swap_volume.assert_called_once_with(guest, 'vdb', conf, 1)
disconnect_volume.assert_called_once_with(self.context,
old_connection_info,
instance)

5
nova/tests/unit/virt/libvirt/test_guest.py
View File

@ -134,11 +134,6 @@ class GuestTestCase(test.NoDBTestCase):
def test_delete_configuration(self):
self.guest.delete_configuration()
self.domain.undefineFlags.assert_called_once_with(
fakelibvirt.VIR_DOMAIN_UNDEFINE_MANAGED_SAVE)
def test_delete_configuration_with_nvram(self):
self.guest.delete_configuration(support_uefi=True)
self.domain.undefineFlags.assert_called_once_with(
fakelibvirt.VIR_DOMAIN_UNDEFINE_MANAGED_SAVE |
fakelibvirt.VIR_DOMAIN_UNDEFINE_NVRAM)

28
nova/virt/libvirt/driver.py
View File

@ -1565,10 +1565,7 @@ class LibvirtDriver(driver.ComputeDriver):
try:
guest = self._host.get_guest(instance)
try:
hw_firmware_type = instance.image_meta.properties.get(
'hw_firmware_type')
support_uefi = self._check_uefi_support(hw_firmware_type)
guest.delete_configuration(support_uefi)
guest.delete_configuration()
except libvirt.libvirtError as e:
with excutils.save_and_reraise_exception() as ctxt:
errcode = e.get_error_code()
@ -2227,7 +2224,7 @@ class LibvirtDriver(driver.ComputeDriver):
self._disconnect_volume(context, connection_info, instance,
encryption=encryption)
def _swap_volume(self, guest, disk_dev, conf, resize_to, hw_firmware_type):
def _swap_volume(self, guest, disk_dev, conf, resize_to):
"""Swap existing disk with a new block device.
Call virDomainBlockRebase or virDomainBlockCopy with Libvirt >= 6.0.0
@ -2237,7 +2234,6 @@ class LibvirtDriver(driver.ComputeDriver):
:param: disk_dev: Device within the domain that is being swapped
:param: conf: LibvirtConfigGuestDisk object representing the new volume
:param: resize_to: Size of the dst volume, 0 if the same as the src
:param: hw_firmware_type: fields.FirmwareType if set in the imagemeta
"""
dev = guest.get_block_device(disk_dev)
@ -2258,8 +2254,7 @@ class LibvirtDriver(driver.ComputeDriver):
# undefine it. If any part of this block fails, the domain is
# re-defined regardless.
if guest.has_persistent_configuration():
support_uefi = self._check_uefi_support(hw_firmware_type)
guest.delete_configuration(support_uefi)
guest.delete_configuration()
try:
dev.copy(conf.to_xml(), reuse_ext=True)
@ -2324,12 +2319,9 @@ class LibvirtDriver(driver.ComputeDriver):
self._connect_volume(context, new_connection_info, instance)
conf = self._get_volume_config(
instance, new_connection_info, disk_info)
hw_firmware_type = instance.image_meta.properties.get(
'hw_firmware_type')
try:
self._swap_volume(guest, disk_dev, conf,
resize_to, hw_firmware_type)
self._swap_volume(guest, disk_dev, conf, resize_to)
except exception.VolumeRebaseFailed:
with excutils.save_and_reraise_exception():
self._disconnect_volume(context, new_connection_info, instance)
@ -3384,10 +3376,7 @@ class LibvirtDriver(driver.ComputeDriver):
# If any part of this block fails, the domain is
# re-defined regardless.
if guest.has_persistent_configuration():
hw_firmware_type = image_meta.properties.get(
'hw_firmware_type')
support_uefi = self._check_uefi_support(hw_firmware_type)
guest.delete_configuration(support_uefi)
guest.delete_configuration()
# NOTE (rmk): Establish a temporary mirror of our root disk and
# issue an abort once we have a complete copy.
@ -6562,13 +6551,6 @@ class LibvirtDriver(driver.ComputeDriver):
return flavor
return instance.flavor
def _check_uefi_support(self, hw_firmware_type):
caps = self._host.get_capabilities()
return self._host.supports_uefi and (
hw_firmware_type == fields.FirmwareType.UEFI or
caps.host.cpu.arch == fields.Architecture.AARCH64
)
def _check_secure_boot_support(
self,
arch: str,

5
nova/virt/libvirt/guest.py
View File

@ -295,12 +295,11 @@ class Guest(object):
yield VCPUInfo(
id=vcpu[0], cpu=vcpu[3], state=vcpu[1], time=vcpu[2])
def delete_configuration(self, support_uefi=False):
def delete_configuration(self):
"""Undefines a domain from hypervisor."""
try:
flags = libvirt.VIR_DOMAIN_UNDEFINE_MANAGED_SAVE
if support_uefi:
flags |= libvirt.VIR_DOMAIN_UNDEFINE_NVRAM
flags |= libvirt.VIR_DOMAIN_UNDEFINE_NVRAM
self._domain.undefineFlags(flags)
except libvirt.libvirtError:
LOG.debug("Error from libvirt during undefineFlags for guest "