From c2ff276c841934ff147aab836a4bd099297fb46b 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. Closes-Bug: 1677621 Change-Id: I578153ca862753ef5b8041ee3853d3c7b2e2be30 --- 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 459731b6bbd8..4cc041882d4a 100644 --- a/nova/network/neutronv2/api.py +++ b/nova/network/neutronv2/api.py @@ -2404,7 +2404,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 1c81fbb0d9ba..3934b033cc7c 100644 --- a/nova/tests/unit/network/test_neutronv2.py +++ b/nova/tests/unit/network/test_neutronv2.py @@ -3861,6 +3861,7 @@ class TestNeutronv2WithMock(test.TestCase): 'pci_vendor_info': 'old_pci_vendor_info'}}, {'id': 'fake-port-2', neutronapi.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 @@ -3868,7 +3869,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( @@ -3915,6 +3916,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 @@ -3923,7 +3925,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)