Merge "Allow driver to properly unplug VIFs on destination on confirm resize" into stable/stein

Author: Zuul

nova/compute/manager.py

@@ -28,6 +28,7 @@
 import base64
 import binascii
 import contextlib
+import copy
 import functools
 import inspect
 import sys
@@ -4025,6 +4026,33 @@ def do_confirm_resize(context, instance, migration_id):
 
         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,
@@ -4043,9 +4071,16 @@ def _confirm_resize(self, context, instance, migration=None):
         # 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)

nova/tests/unit/compute/test_compute.py

@@ -5814,6 +5814,8 @@ def test_confirm_resize_with_numa_topology_and_cpu_pinning(
         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 @@ def _test_resize_with_pci(self, method, expected_pci_addr):
         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,14 +8274,18 @@ def test_confirm_resize_roll_back_quota_status_dummy(self,
         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')
 
         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

nova/tests/unit/compute/test_compute_mgr.py

@@ -7294,7 +7294,9 @@ def do_confirm_resize(mock_save, mock_drop, mock_delete,
                               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'
@@ -7310,6 +7312,7 @@ def do_confirm_resize(mock_save, mock_drop, mock_delete,
 
         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')
@@ -7318,24 +7321,27 @@ def do_confirm_resize(mock_save, mock_drop, mock_delete,
     @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'.
         """
         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)
         with test.nested(
             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,
@@ -7362,6 +7368,59 @@ def test_confirm_resize_driver_confirm_migration_fails(
         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
@@ -8757,6 +8816,24 @@ def doit(mock_pr, mock_r):
 
         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):