From 70c1eb689ad174b61ad915ae5384778bd536c16c Mon Sep 17 00:00:00 2001 From: Steven Webster Date: Mon, 27 Mar 2017 12:18:23 -0400 Subject: [PATCH] Fix port update exception when unshelving an instance with PCI devices It is possible that _update_port_binding_for_instance() is called without a migration object, such as when a user unshelves an instance. If the instance has a port(s) with a PCI device binding, the current logic extracts a pci mapping from old to new devices from the migration object and migration context. If a 'new' device is not found in the PCI mapping, an exception is thrown. In the case of an unshelve, there is no migration object (or migration context), and as such we have an empty pci mapping. This fix will only check for a new device if we have a migration object. Conflicts: nova/tests/unit/network/test_neutronv2.py NOTE(mriedem): The conflict is due to not having change I818d2232f3398489be6303414585840c151e4db7 in Newton. Closes-Bug: 1677621 Change-Id: I578153ca862753ef5b8041ee3853d3c7b2e2be30 (cherry picked from commit c2ff276c841934ff147aab836a4bd099297fb46b) (cherry picked from commit f281c18ba9aea1b2e8a36d5ae91a7acc5324ac5e) --- nova/network/neutronv2/api.py | 3 +- nova/tests/unit/network/test_neutronv2.py | 54 ++++++++++++++++++++++- 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/nova/network/neutronv2/api.py b/nova/network/neutronv2/api.py index 9eac7248ebe7..05c409323624 100644 --- a/nova/network/neutronv2/api.py +++ b/nova/network/neutronv2/api.py @@ -2393,7 +2393,8 @@ class API(base_api.NetworkAPI): # resize is happening on the same host, a new PCI device can be # allocated. vnic_type = p.get('binding:vnic_type') - if vnic_type in network_model.VNIC_TYPES_SRIOV: + if (vnic_type in network_model.VNIC_TYPES_SRIOV + and migration is not None): if not pci_mapping: pci_mapping = self._get_pci_mapping_for_migration(context, instance, migration) diff --git a/nova/tests/unit/network/test_neutronv2.py b/nova/tests/unit/network/test_neutronv2.py index f2e7abcb790b..9ace114057a1 100644 --- a/nova/tests/unit/network/test_neutronv2.py +++ b/nova/tests/unit/network/test_neutronv2.py @@ -3811,6 +3811,7 @@ class TestNeutronv2WithMock(test.TestCase): 'pci_vendor_info': 'old_pci_vendor_info'}}, {'id': 'fake-port-2', 'binding:host_id': instance.host}]} + migration = {'status': 'confirmed'} list_ports_mock = mock.Mock(return_value=fake_ports) get_client_mock.return_value.list_ports = list_ports_mock @@ -3818,7 +3819,7 @@ class TestNeutronv2WithMock(test.TestCase): get_client_mock.return_value.update_port = update_port_mock self.api._update_port_binding_for_instance(self.context, instance, - instance.host) + instance.host, migration) # Assert that update_port is called with the binding:profile # corresponding to the PCI device specified. update_port_mock.assert_called_once_with( @@ -3864,6 +3865,7 @@ class TestNeutronv2WithMock(test.TestCase): {'pci_slot': '0000:0a:00.1', 'physical_network': 'old_phys_net', 'pci_vendor_info': 'old_pci_vendor_info'}}]} + migration = {'status': 'confirmed'} list_ports_mock = mock.Mock(return_value=fake_ports) get_client_mock.return_value.list_ports = list_ports_mock @@ -3872,7 +3874,55 @@ class TestNeutronv2WithMock(test.TestCase): self.api._update_port_binding_for_instance, self.context, instance, - instance.host) + instance.host, + migration) + + @mock.patch.object(pci_whitelist.Whitelist, 'get_devspec') + @mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock()) + def test_update_port_bindings_for_instance_with_pci_no_migration(self, + get_client_mock, + get_pci_device_devspec_mock): + self.api._has_port_binding_extension = mock.Mock(return_value=True) + + devspec = mock.Mock() + devspec.get_tags.return_value = {'physical_network': 'physnet1'} + get_pci_device_devspec_mock.return_value = devspec + + instance = fake_instance.fake_instance_obj(self.context) + instance.migration_context = objects.MigrationContext() + instance.migration_context.old_pci_devices = objects.PciDeviceList( + objects=[objects.PciDevice(vendor_id='1377', + product_id='0047', + address='0000:0a:00.1', + compute_node_id=1, + request_id='1234567890')]) + instance.migration_context.new_pci_devices = objects.PciDeviceList( + objects=[objects.PciDevice(vendor_id='1377', + product_id='0047', + address='0000:0b:00.1', + compute_node_id=2, + request_id='1234567890')]) + instance.pci_devices = instance.migration_context.old_pci_devices + + fake_ports = {'ports': [ + {'id': 'fake-port-1', + 'binding:vnic_type': 'direct', + neutronapi.BINDING_HOST_ID: instance.host, + neutronapi.BINDING_PROFILE: + {'pci_slot': '0000:0a:00.1', + 'physical_network': 'phys_net', + 'pci_vendor_info': 'pci_vendor_info'}}]} + list_ports_mock = mock.Mock(return_value=fake_ports) + get_client_mock.return_value.list_ports = list_ports_mock + + update_port_mock = mock.Mock() + get_client_mock.return_value.update_port = update_port_mock + + # Try to update the port binding with no migration object. + self.api._update_port_binding_for_instance(self.context, instance, + instance.host) + # No ports should be updated if the port's pci binding did not change. + update_port_mock.assert_not_called() def test_get_pci_mapping_for_migration(self): instance = fake_instance.fake_instance_obj(self.context)