Allow driver to properly unplug VIFs on destination on confirm resize
Update PCI related VIFs with the original PCI address on the source
host to allow the virt driver to properly unplug VIFs from
hypervisor, e.g allow the proper VF representor to be unplugged
from the integration bridge in case of a hardware offloaded OVS.
While other approaches are possible for solving the issue,
The approach proposed in the series allows the fix to be safely
backported.
Closes-Bug: #1809095
Conflicts:
nova/tests/unit/compute/test_compute_mgr.py
Change-Id: Id3c4d839fb1a6da47cfb366b65c0904d281a218f
(cherry picked from commit 000e93df09)
This commit is contained in:
Adrian Chiris
parent
84bb00a86d
commit
77a339c4e8
|
|
@ -28,6 +28,7 @@ terminating it.
|
|||
import base64
|
||||
import binascii
|
||||
import contextlib
|
||||
import copy
|
||||
import functools
|
||||
import inspect
|
||||
import sys
|
||||
|
|
@ -4005,6 +4006,33 @@ class ComputeManager(manager.Manager):
|
|||
|
||||
do_confirm_resize(context, instance, migration.id)
|
||||
|
||||
def _get_updated_nw_info_with_pci_mapping(self, nw_info, pci_mapping):
|
||||
# NOTE(adrianc): This method returns a copy of nw_info if modifications
|
||||
# are made else it returns the original nw_info.
|
||||
updated_nw_info = nw_info
|
||||
if nw_info and pci_mapping:
|
||||
updated_nw_info = copy.deepcopy(nw_info)
|
||||
for vif in updated_nw_info:
|
||||
if vif['vnic_type'] in network_model.VNIC_TYPES_SRIOV:
|
||||
try:
|
||||
vif_pci_addr = vif['profile']['pci_slot']
|
||||
new_addr = pci_mapping[vif_pci_addr].address
|
||||
vif['profile']['pci_slot'] = new_addr
|
||||
LOG.debug("Updating VIF's PCI address for VIF %(id)s. "
|
||||
"Original value %(orig_val)s, "
|
||||
"new value %(new_val)s",
|
||||
{'id': vif['id'],
|
||||
'orig_val': vif_pci_addr,
|
||||
'new_val': new_addr})
|
||||
except (KeyError, AttributeError):
|
||||
with excutils.save_and_reraise_exception():
|
||||
# NOTE(adrianc): This should never happen. If we
|
||||
# get here it means there is some inconsistency
|
||||
# with either 'nw_info' or 'pci_mapping'.
|
||||
LOG.error("Unexpected error when updating network "
|
||||
"information with PCI mapping.")
|
||||
return updated_nw_info
|
||||
|
||||
def _confirm_resize(self, context, instance, migration=None):
|
||||
"""Destroys the source instance."""
|
||||
self._notify_about_instance_usage(context, instance,
|
||||
|
|
@ -4023,9 +4051,16 @@ class ComputeManager(manager.Manager):
|
|||
# NOTE(tr3buchet): tear down networks on source host
|
||||
self.network_api.setup_networks_on_host(context, instance,
|
||||
migration.source_compute, teardown=True)
|
||||
|
||||
network_info = self.network_api.get_instance_nw_info(context,
|
||||
instance)
|
||||
|
||||
# NOTE(adrianc): Populate old PCI device in VIF profile
|
||||
# to allow virt driver to properly unplug it from Hypervisor.
|
||||
pci_mapping = (instance.migration_context.
|
||||
get_pci_mapping_for_migration(True))
|
||||
network_info = self._get_updated_nw_info_with_pci_mapping(
|
||||
network_info, pci_mapping)
|
||||
|
||||
# TODO(mriedem): Get BDMs here and pass them to the driver.
|
||||
self.driver.confirm_migration(context, migration, instance,
|
||||
network_info)
|
||||
|
|
|
|||
|
|
@ -5814,6 +5814,8 @@ class ComputeTestCase(BaseTestCase,
|
|||
migration_context.migration_id = migration.id
|
||||
migration_context.old_numa_topology = old_inst_topology
|
||||
migration_context.new_numa_topology = new_inst_topology
|
||||
migration_context.old_pci_devices = None
|
||||
migration_context.new_pci_devices = None
|
||||
|
||||
instance.migration_context = migration_context
|
||||
instance.vm_state = vm_states.RESIZED
|
||||
|
|
@ -5850,12 +5852,14 @@ class ComputeTestCase(BaseTestCase,
|
|||
old_pci_devices = objects.PciDeviceList(
|
||||
objects=[objects.PciDevice(vendor_id='1377',
|
||||
product_id='0047',
|
||||
address='0000:0a:00.1')])
|
||||
address='0000:0a:00.1',
|
||||
request_id=uuids.req1)])
|
||||
|
||||
new_pci_devices = objects.PciDeviceList(
|
||||
objects=[objects.PciDevice(vendor_id='1377',
|
||||
product_id='0047',
|
||||
address='0000:0b:00.1')])
|
||||
address='0000:0b:00.1',
|
||||
request_id=uuids.req2)])
|
||||
|
||||
if expected_pci_addr == old_pci_devices[0].address:
|
||||
expected_pci_device = old_pci_devices[0]
|
||||
|
|
@ -8270,7 +8274,10 @@ class ComputeTestCase(BaseTestCase,
|
|||
self.compute.confirm_resize(self.context, instance=instance,
|
||||
migration=migration)
|
||||
|
||||
def test_allow_confirm_resize_on_instance_in_deleting_task_state(self):
|
||||
@mock.patch.object(objects.MigrationContext,
|
||||
'get_pci_mapping_for_migration')
|
||||
def test_allow_confirm_resize_on_instance_in_deleting_task_state(
|
||||
self, mock_pci_mapping):
|
||||
instance = self._create_fake_instance_obj()
|
||||
old_type = instance.flavor
|
||||
new_type = flavors.get_flavor_by_flavor_id('4')
|
||||
|
|
@ -8278,6 +8285,7 @@ class ComputeTestCase(BaseTestCase,
|
|||
instance.flavor = new_type
|
||||
instance.old_flavor = old_type
|
||||
instance.new_flavor = new_type
|
||||
instance.migration_context = objects.MigrationContext()
|
||||
|
||||
def fake_drop_move_claim(*args, **kwargs):
|
||||
pass
|
||||
|
|
|
|||
|
|
@ -7208,7 +7208,9 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase,
|
|||
mock_confirm, mock_nwapi, mock_notify,
|
||||
mock_mig_save, mock_mig_get, mock_inst_get):
|
||||
self._mock_rt()
|
||||
self.instance.migration_context = objects.MigrationContext()
|
||||
self.instance.migration_context = objects.MigrationContext(
|
||||
new_pci_devices=None,
|
||||
old_pci_devices=None)
|
||||
self.migration.source_compute = self.instance['host']
|
||||
self.migration.source_node = self.instance['node']
|
||||
self.migration.status = 'confirming'
|
||||
|
|
@ -7224,6 +7226,7 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase,
|
|||
|
||||
do_confirm_resize()
|
||||
|
||||
@mock.patch('nova.objects.MigrationContext.get_pci_mapping_for_migration')
|
||||
@mock.patch('nova.compute.utils.add_instance_fault_from_exc')
|
||||
@mock.patch('nova.objects.Migration.get_by_id')
|
||||
@mock.patch('nova.objects.Instance.get_by_uuid')
|
||||
|
|
@ -7232,7 +7235,7 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase,
|
|||
@mock.patch('nova.objects.Instance.save')
|
||||
def test_confirm_resize_driver_confirm_migration_fails(
|
||||
self, instance_save, notify_action, notify_usage,
|
||||
instance_get_by_uuid, migration_get_by_id, add_fault):
|
||||
instance_get_by_uuid, migration_get_by_id, add_fault, get_mapping):
|
||||
"""Tests the scenario that driver.confirm_migration raises some error
|
||||
to make sure the error is properly handled, like the instance and
|
||||
migration status is set to 'error'.
|
||||
|
|
@ -7240,6 +7243,7 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase,
|
|||
self.migration.status = 'confirming'
|
||||
migration_get_by_id.return_value = self.migration
|
||||
instance_get_by_uuid.return_value = self.instance
|
||||
self.instance.migration_context = objects.MigrationContext()
|
||||
|
||||
error = exception.HypervisorUnavailable(
|
||||
host=self.migration.source_compute)
|
||||
|
|
@ -7247,9 +7251,11 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase,
|
|||
mock.patch.object(self.compute, 'network_api'),
|
||||
mock.patch.object(self.compute.driver, 'confirm_migration',
|
||||
side_effect=error),
|
||||
mock.patch.object(self.compute, '_delete_allocation_after_move')
|
||||
mock.patch.object(self.compute, '_delete_allocation_after_move'),
|
||||
mock.patch.object(self.compute,
|
||||
'_get_updated_nw_info_with_pci_mapping')
|
||||
) as (
|
||||
network_api, confirm_migration, delete_allocation
|
||||
network_api, confirm_migration, delete_allocation, pci_mapping
|
||||
):
|
||||
self.assertRaises(exception.HypervisorUnavailable,
|
||||
self.compute.confirm_resize,
|
||||
|
|
@ -7276,6 +7282,59 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase,
|
|||
instance_get_by_uuid.assert_called_once()
|
||||
migration_get_by_id.assert_called_once()
|
||||
|
||||
def test_confirm_resize_calls_virt_driver_with_old_pci(self):
|
||||
@mock.patch.object(self.migration, 'save')
|
||||
@mock.patch.object(self.compute, '_notify_about_instance_usage')
|
||||
@mock.patch.object(self.compute, 'network_api')
|
||||
@mock.patch.object(self.compute.driver, 'confirm_migration')
|
||||
@mock.patch.object(self.compute, '_delete_allocation_after_move')
|
||||
@mock.patch.object(self.instance, 'drop_migration_context')
|
||||
@mock.patch.object(self.instance, 'save')
|
||||
def do_confirm_resize(mock_save, mock_drop, mock_delete,
|
||||
mock_confirm, mock_nwapi, mock_notify,
|
||||
mock_mig_save):
|
||||
# Mock virt driver confirm_resize() to save the provided
|
||||
# network_info, we will check it later.
|
||||
updated_nw_info = []
|
||||
|
||||
def driver_confirm_resize(*args, **kwargs):
|
||||
if 'network_info' in kwargs:
|
||||
nw_info = kwargs['network_info']
|
||||
else:
|
||||
nw_info = args[3]
|
||||
updated_nw_info.extend(nw_info)
|
||||
|
||||
mock_confirm.side_effect = driver_confirm_resize
|
||||
self._mock_rt()
|
||||
old_devs = objects.PciDeviceList(
|
||||
objects=[objects.PciDevice(
|
||||
address='0000:04:00.2',
|
||||
request_id=uuids.pcidev1)])
|
||||
new_devs = objects.PciDeviceList(
|
||||
objects=[objects.PciDevice(
|
||||
address='0000:05:00.3',
|
||||
request_id=uuids.pcidev1)])
|
||||
self.instance.migration_context = objects.MigrationContext(
|
||||
new_pci_devices=new_devs,
|
||||
old_pci_devices=old_devs)
|
||||
# Create VIF with new_devs[0] PCI address.
|
||||
nw_info = network_model.NetworkInfo([
|
||||
network_model.VIF(
|
||||
id=uuids.port1,
|
||||
vnic_type=network_model.VNIC_TYPE_DIRECT,
|
||||
profile={'pci_slot': new_devs[0].address})])
|
||||
mock_nwapi.get_instance_nw_info.return_value = nw_info
|
||||
self.migration.source_compute = self.instance['host']
|
||||
self.migration.source_node = self.instance['node']
|
||||
self.compute._confirm_resize(self.context, self.instance,
|
||||
self.migration)
|
||||
# Assert virt driver confirm_migration() was called
|
||||
# with the updated nw_info object.
|
||||
self.assertEqual(old_devs[0].address,
|
||||
updated_nw_info[0]['profile']['pci_slot'])
|
||||
|
||||
do_confirm_resize()
|
||||
|
||||
def test_delete_allocation_after_move_confirm_by_migration(self):
|
||||
with mock.patch.object(self.compute, 'reportclient') as mock_report:
|
||||
mock_report.delete_allocation_for_instance.return_value = True
|
||||
|
|
@ -8671,6 +8730,24 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase,
|
|||
|
||||
doit()
|
||||
|
||||
def test_get_updated_nw_info_with_pci_mapping(self):
|
||||
old_dev = objects.PciDevice(address='0000:04:00.2')
|
||||
new_dev = objects.PciDevice(address='0000:05:00.3')
|
||||
pci_mapping = {old_dev.address: new_dev}
|
||||
nw_info = network_model.NetworkInfo([
|
||||
network_model.VIF(
|
||||
id=uuids.port1,
|
||||
vnic_type=network_model.VNIC_TYPE_NORMAL),
|
||||
network_model.VIF(
|
||||
id=uuids.port2,
|
||||
vnic_type=network_model.VNIC_TYPE_DIRECT,
|
||||
profile={'pci_slot': old_dev.address})])
|
||||
updated_nw_info = self.compute._get_updated_nw_info_with_pci_mapping(
|
||||
nw_info, pci_mapping)
|
||||
self.assertDictEqual(nw_info[0], updated_nw_info[0])
|
||||
self.assertEqual(new_dev.address,
|
||||
updated_nw_info[1]['profile']['pci_slot'])
|
||||
|
||||
|
||||
class ComputeManagerInstanceUsageAuditTestCase(test.TestCase):
|
||||
def setUp(self):
|
||||
|
|
|
|||
Loading…
Reference in New Issue