diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 386e8c0cbeb1..b994957879df 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -16764,15 +16764,17 @@ class LibvirtConnTestCase(test.NoDBTestCase, instance.system_metadata) self.assertTrue(mock_attachDevice.called) - @mock.patch.object(host.Host, - 'has_min_version', return_value=True) - def _test_detach_direct_passthrough_ports(self, - mock_has_min_version, vif_type): + @mock.patch.object( + host.Host, 'has_min_version', new=mock.Mock(return_value=True) + ) + def _test_detach_direct_passthrough_ports( + self, vif_type, detach_device=True, + vnic_type=network_model.VNIC_TYPE_DIRECT): instance = objects.Instance(**self.test_instance) expeted_pci_slot = "0000:00:00.0" network_info = _fake_network_info(self) - network_info[0]['vnic_type'] = network_model.VNIC_TYPE_DIRECT + network_info[0]['vnic_type'] = vnic_type # some more adjustments for the fake network_info so that # the correct get_config function will be executed (vif's # get_config_hw_veb - which is according to the real SRIOV vif) @@ -16788,29 +16790,52 @@ class LibvirtConnTestCase(test.NoDBTestCase, # pci_manager.get_instance_pci_devs will not return an empty list # which will eventually fail the assertion for detachDeviceFlags expected_pci_device_obj = ( - objects.PciDevice(address=expeted_pci_slot, request_id=None)) + objects.PciDevice( + address=expeted_pci_slot, request_id=None, compute_node_id=42 + ) + ) instance.pci_devices = objects.PciDeviceList() instance.pci_devices.objects = [expected_pci_device_obj] - domain = FakeVirtDomain() + domain = FakeVirtDomain(id=24601, name='Jean Valjean') drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) guest = libvirt_guest.Guest(domain) - with mock.patch.object(drvr, '_detach_pci_devices') as mock_detach_pci: + with mock.patch.object( + drvr, '_detach_pci_devices' + ) as mock_detach_pci, mock.patch.object( + drvr, 'detach_interface' + ) as mock_detach_interface: drvr._detach_direct_passthrough_ports( self.context, instance, guest) - mock_detach_pci.assert_called_once_with( - guest, [expected_pci_device_obj]) + if detach_device: + mock_detach_pci.assert_called_once_with( + guest, [expected_pci_device_obj]) + else: + mock_detach_interface.assert_called_once() - def test_detach_direct_passthrough_ports_interface_interface_hostdev(self): + def test_detach_direct_passthrough_ports_ovs_hw_offload(self): # Note: test detach_direct_passthrough_ports method for vif with config # LibvirtConfigGuestInterface - self._test_detach_direct_passthrough_ports(vif_type="hw_veb") + self._test_detach_direct_passthrough_ports("ovs", detach_device=False) - def test_detach_direct_passthrough_ports_interface_pci_hostdev(self): + def test_detach_direct_passthrough_ports_sriov_nic_agent(self): + # Note: test detach_direct_passthrough_ports method for vif with config + # LibvirtConfigGuestInterface + self._test_detach_direct_passthrough_ports( + "hw_veb", detach_device=False + ) + + def test_detach_direct_physical_passthrough_ports_sriov_nic_agent(self): + self._test_detach_direct_passthrough_ports( + "hostdev_physical", + vnic_type=network_model.VNIC_TYPE_DIRECT_PHYSICAL + ) + + def test_detach_direct_passthrough_ports_infiniband(self): # Note: test detach_direct_passthrough_ports method for vif with config # LibvirtConfigGuestHostdevPCI - self._test_detach_direct_passthrough_ports(vif_type="ib_hostdev") + self._test_detach_direct_passthrough_ports("ib_hostdev") @mock.patch.object(host.Host, 'has_min_version', return_value=True) @mock.patch.object(FakeVirtDomain, 'detachDeviceFlags') @@ -16820,9 +16845,10 @@ class LibvirtConnTestCase(test.NoDBTestCase, network_info = _fake_network_info(self, 2) + direct_physical = network_model.VNIC_TYPE_DIRECT_PHYSICAL for network_info_inst in network_info: - network_info_inst['vnic_type'] = network_model.VNIC_TYPE_DIRECT - network_info_inst['type'] = "hw_veb" + network_info_inst['vnic_type'] = direct_physical + network_info_inst['type'] = "hostdev_physical" network_info_inst['details'] = dict(vlan="2145") network_info_inst['address'] = "fa:16:3e:96:2a:48" diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index d9cf1f203d4a..134775c01b62 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -4982,16 +4982,18 @@ class LibvirtDriver(driver.ComputeDriver): guest_config = vconfig.LibvirtConfigGuest() guest_config.parse_dom(xml_doc) - for hdev in [d for d in guest_config.devices - if isinstance(d, vconfig.LibvirtConfigGuestHostdevPCI)]: + for hdev in [ + d for d in guest_config.devices + if isinstance(d, vconfig.LibvirtConfigGuestHostdevPCI) + ]: hdbsf = [hdev.domain, hdev.bus, hdev.slot, hdev.function] dbsf = pci_utils.parse_address(dev.address) - if [int(x, 16) for x in hdbsf] ==\ - [int(x, 16) for x in dbsf]: - raise exception.PciDeviceDetachFailed(reason= - "timeout", - dev=dev) - + if ( + [int(x, 16) for x in hdbsf] == + [int(x, 16) for x in dbsf] + ): + raise exception.PciDeviceDetachFailed( + reason="timeout", dev=dev) except libvirt.libvirtError as ex: error_code = ex.get_error_code() if error_code == libvirt.VIR_ERR_NO_DOMAIN: @@ -5039,33 +5041,76 @@ class LibvirtDriver(driver.ComputeDriver): instance=instance) guest.attach_device(cfg) + # TODO(sean-k-mooney): we should try and converge this fuction with + # _detach_direct_passthrough_vifs which does the same operation correctly + # for live migration def _detach_direct_passthrough_ports(self, context, instance, guest): network_info = instance.info_cache.network_info if network_info is None: return if self._has_direct_passthrough_port(network_info): - # In case of VNIC_TYPES_DIRECT_PASSTHROUGH ports we create - # pci request per direct passthrough port. Therefore we can trust - # that pci_slot value in the vif is correct. - direct_passthrough_pci_addresses = [ + + attached_via_hostdev_element = [] + attached_via_interface_element = [] + + for vif in network_info: + if vif['profile'].get('pci_slot') is None: + # this is not an sriov interface so skip it + continue + + if (vif['vnic_type'] not in + network_model.VNIC_TYPES_DIRECT_PASSTHROUGH): + continue + + cfg = self.vif_driver.get_config( + instance, vif, instance.image_meta, instance.flavor, + CONF.libvirt.virt_type) + LOG.debug(f'Detaching type: {type(cfg)}, data: {cfg}') + if isinstance(cfg, vconfig.LibvirtConfigGuestHostdevPCI): + attached_via_hostdev_element.append(vif) + else: + attached_via_interface_element.append(vif) + + pci_devs = pci_manager.get_instance_pci_devs(instance, 'all') + hostdev_pci_addresses = { vif['profile']['pci_slot'] - for vif in network_info - if (vif['vnic_type'] in - network_model.VNIC_TYPES_DIRECT_PASSTHROUGH and - vif['profile'].get('pci_slot') is not None) + for vif in attached_via_hostdev_element + } + direct_passthrough_pci_addresses = [ + pci_dev for pci_dev in pci_devs + if pci_dev.address in hostdev_pci_addresses ] - # use detach_pci_devices to avoid failure in case of - # multiple guest direct passthrough ports with the same MAC - # (protection use-case, ports are on different physical - # interfaces) - pci_devs = pci_manager.get_instance_pci_devs(instance, 'all') - direct_passthrough_pci_addresses = ( - [pci_dev for pci_dev in pci_devs - if pci_dev.address in direct_passthrough_pci_addresses]) + # FIXME(sean-k-mooney): i am using _detach_pci_devices because + # of the previous comment introduced by change-id: + # I3a45b1fb41e8e446d1f25d7a1d77991c8bf2a1ed + # in relation to bug 1563874 however i'm not convinced that + # patch was correct so we should reevaluate if we should do this. + # The intent of using _detach_pci_devices is + # to somehow cater for the use case where multiple ports have + # the same MAC address however _detach_pci_device can only remove + # device that are attached as hostdev elements, not via the + # interface element. + # So using it for all devices would break vnic-type direct when + # using the sriov_nic_agent ml2 driver or vif of vnic_type vdpa. + # Since PF ports cant have the same MAC that means that this + # use case was for hardware offloaded OVS? many NICs do not allow + # two VFs to have the same MAC on different VLANs due to the + # ordering of the VLAN and MAC filters in there static packet + # processing pipeline as such its unclear if this will work in any + # non ovs offload case. We should look into this more closely + # as from my testing in this patch we appear to use the interface + # element for hardware offloaded ovs too. Infiniband and vnic_type + # direct-physical port type do need this code path, both those cant + # have duplicate MACs... self._detach_pci_devices(guest, direct_passthrough_pci_addresses) + # for ports that are attached with interface elements we cannot use + # _detach_pci_devices so we use detach_interface + for vif in attached_via_interface_element: + self.detach_interface(context, instance, vif) + def _update_compute_provider_status(self, context, service): """Calls the ComputeVirtAPI.update_compute_provider_status method