diff --git a/mypy-files.txt b/mypy-files.txt index a2085f8f4a26..ad27eeb620c5 100644 --- a/mypy-files.txt +++ b/mypy-files.txt @@ -3,3 +3,4 @@ nova/virt/driver.py nova/virt/hardware.py nova/virt/libvirt/__init__.py nova/virt/libvirt/driver.py +nova/virt/libvirt/host.py diff --git a/nova/pci/utils.py b/nova/pci/utils.py index 5b0a0821f944..52ca20a616b1 100644 --- a/nova/pci/utils.py +++ b/nova/pci/utils.py @@ -200,27 +200,3 @@ def get_vf_num_by_pci_address(pci_addr): if vf_num is None: raise exception.PciDeviceNotFoundById(id=pci_addr) return vf_num - - -def get_net_name_by_vf_pci_address(vfaddress): - """Given the VF PCI address, returns the net device name. - - Every VF is associated to a PCI network device. This function - returns the libvirt name given to this network device; e.g.: - - - net_enp8s0f0_90_e2_ba_5e_a6_40 - ... - - In the libvirt parser information tree, the network device stores the - network capabilities associated to this device. - """ - try: - mac = get_mac_by_pci_address(vfaddress).split(':') - ifname = get_ifname_by_pci_address(vfaddress) - return ("net_%(ifname)s_%(mac)s" % - {'ifname': ifname, 'mac': '_'.join(mac)}) - except Exception: - LOG.warning("No net device was found for VF %(vfaddress)s", - {'vfaddress': vfaddress}) - return diff --git a/nova/tests/unit/pci/test_utils.py b/nova/tests/unit/pci/test_utils.py index d6e004c9b9ea..51ebdbfe0c6e 100644 --- a/nova/tests/unit/pci/test_utils.py +++ b/nova/tests/unit/pci/test_utils.py @@ -266,46 +266,3 @@ class GetVfNumByPciAddressTestCase(test.NoDBTestCase): utils.get_vf_num_by_pci_address, self.pci_address ) - - -class GetNetNameByVfPciAddressTestCase(test.NoDBTestCase): - - def setUp(self): - super(GetNetNameByVfPciAddressTestCase, self).setUp() - self._get_mac = mock.patch.object(utils, 'get_mac_by_pci_address') - self.mock_get_mac = self._get_mac.start() - self._get_ifname = mock.patch.object( - utils, 'get_ifname_by_pci_address') - self.mock_get_ifname = self._get_ifname.start() - self.addCleanup(self._get_mac.stop) - self.addCleanup(self._get_ifname.stop) - - self.mac = 'ca:fe:ca:fe:ca:fe' - self.if_name = 'enp7s0f0' - self.pci_address = '0000:07:02.1' - - def test_correct_behaviour(self): - ref_net_name = 'net_enp7s0f0_ca_fe_ca_fe_ca_fe' - self.mock_get_mac.return_value = self.mac - self.mock_get_ifname.return_value = self.if_name - net_name = utils.get_net_name_by_vf_pci_address(self.pci_address) - self.assertEqual(ref_net_name, net_name) - self.mock_get_mac.assert_called_once_with(self.pci_address) - self.mock_get_ifname.assert_called_once_with(self.pci_address) - - def test_wrong_mac(self): - self.mock_get_mac.side_effect = ( - exception.PciDeviceNotFoundById(self.pci_address)) - net_name = utils.get_net_name_by_vf_pci_address(self.pci_address) - self.assertIsNone(net_name) - self.mock_get_mac.assert_called_once_with(self.pci_address) - self.mock_get_ifname.assert_not_called() - - def test_wrong_ifname(self): - self.mock_get_mac.return_value = self.mac - self.mock_get_ifname.side_effect = ( - exception.PciDeviceNotFoundById(self.pci_address)) - net_name = utils.get_net_name_by_vf_pci_address(self.pci_address) - self.assertIsNone(net_name) - self.mock_get_mac.assert_called_once_with(self.pci_address) - self.mock_get_ifname.assert_called_once_with(self.pci_address) diff --git a/nova/tests/unit/virt/libvirt/fakelibvirt.py b/nova/tests/unit/virt/libvirt/fakelibvirt.py index d2e59d9a5f42..f303e4761517 100644 --- a/nova/tests/unit/virt/libvirt/fakelibvirt.py +++ b/nova/tests/unit/virt/libvirt/fakelibvirt.py @@ -15,6 +15,7 @@ import sys import textwrap import time +import typing as ty import fixtures from lxml import etree @@ -167,6 +168,10 @@ VIR_DOMAIN_BLOCK_COMMIT_RELATIVE = 4 VIR_CONNECT_LIST_DOMAINS_ACTIVE = 1 VIR_CONNECT_LIST_DOMAINS_INACTIVE = 2 +# virConnectListAllNodeDevices flags +VIR_CONNECT_LIST_NODE_DEVICES_CAP_PCI_DEV = 2 +VIR_CONNECT_LIST_NODE_DEVICES_CAP_NET = 16 + # secret type VIR_SECRET_USAGE_TYPE_NONE = 0 VIR_SECRET_USAGE_TYPE_VOLUME = 1 @@ -220,11 +225,12 @@ class FakePCIDevice(object): - X540 Ethernet Controller Virtual Function (8086:1515) """ + pci_default_parent = "pci_0000_80_01_0" pci_device_template = textwrap.dedent(""" pci_0000_81_%(slot)02x_%(function)d /sys/devices/pci0000:80/0000:80:01.0/0000:81:%(slot)02x.%(function)d - pci_0000_80_01_0 + %(parent)s %(driver)s @@ -257,7 +263,7 @@ class FakePCIDevice(object): is_capable_of_mdevs = False def __init__(self, dev_type, slot, function, iommu_group, numa_node, - vf_ratio=None, multiple_gpu_types=False): + vf_ratio=None, multiple_gpu_types=False, parent=None): """Populate pci devices :param dev_type: (string) Indicates the type of the device (PCI, PF, @@ -347,6 +353,7 @@ class FakePCIDevice(object): 'capability': capability, 'iommu_group': iommu_group, 'numa_node': numa_node, + 'parent': parent or self.pci_default_parent } # -1 is the sentinel set in /sys/bus/pci/devices/*/numa_node # for no NUMA affinity. When the numa_node is set to -1 on a device @@ -449,7 +456,7 @@ class HostPCIDevicesInfo(object): iommu_group=iommu_group, numa_node=numa_node_pf, vf_ratio=vf_ratio) - + pf_dev_name = pci_dev_name # Generate VFs for _ in range(vf_ratio): function += 1 @@ -471,7 +478,8 @@ class HostPCIDevicesInfo(object): function=function, iommu_group=iommu_group, numa_node=numa_node_pf, - vf_ratio=vf_ratio) + vf_ratio=vf_ratio, + parent=pf_dev_name) slot += 1 @@ -799,7 +807,8 @@ class NodeDevice(object): def _parse_xml(self, xml): tree = etree.fromstring(xml) root = tree.find('.') - self._name = root.get('name') + self._name = root.find('name').text + self._parent = root.find('parent').text def attach(self): pass @@ -810,6 +819,18 @@ class NodeDevice(object): def reset(self): pass + def XMLDesc(self, flags: int) -> str: + return self._xml + + def parent(self) -> str: + return self._parent + + def name(self) -> str: + return self._name + + def listCaps(self) -> ty.List[str]: + return [self.name().split('_')[0]] + class Domain(object): def __init__(self, connection, xml, running=False, transient=False): @@ -1711,6 +1732,13 @@ class Connection(object): def secretDefineXML(self, xml): pass + def listAllDevices(self, flags): + # Note this is incomplete as we do not filter + # based on the flags however it is enough for our + # current testing. + return [NodeDevice(self, xml=dev.XMLDesc(0)) + for dev in self.pci_info.devices.values()] + def openAuth(uri, auth, flags=0): diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 2b35392aae90..1a9c111b0bc6 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -128,8 +128,8 @@ CONF = nova.conf.CONF _fake_network_info = fake_network.fake_get_instance_nw_info -_fake_NodeDevXml = \ - {"pci_0000_04_00_3": """ +_fake_NodeDevXml = { + "pci_0000_04_00_3": """ pci_0000_04_00_3 pci_0000_00_01_1 @@ -154,7 +154,7 @@ _fake_NodeDevXml = \ "pci_0000_04_10_7": """ pci_0000_04_10_7 - pci_0000_00_01_1 + pci_0000_04_00_3 igbvf @@ -176,7 +176,7 @@ _fake_NodeDevXml = \ "pci_0000_04_11_7": """ pci_0000_04_11_7 - pci_0000_00_01_1 + pci_0000_04_00_3 igbvf @@ -281,6 +281,26 @@ _fake_NodeDevXml = \ + """, + "net_enp2s1_02_9a_a1_37_be_54": """ + + net_enp2s1_02_9a_a1_37_be_54 + /sys/devices/pci0000:00/0000:04:00.3/0000:04:10.7/net/enp2s1 + pci_0000_04_10_7 + + enp2s1 +
02:9a:a1:37:be:54
+ + + + + + + + + + + """, "net_enp2s2_02_9a_a1_37_be_54": """ @@ -343,6 +363,15 @@ _fake_NodeDevXml = \ """, } +_fake_NodeDevXml_parents = { + name: etree.fromstring(xml).find("parent").text + for name, xml in _fake_NodeDevXml.items() +} + +_fake_NodeDevXml_children = defaultdict(list) +for key, val in _fake_NodeDevXml_parents.items(): + _fake_NodeDevXml_children[val].append(key) + _fake_cpu_info = { "arch": "test_arch", "model": "test_model", @@ -17225,70 +17254,55 @@ class LibvirtConnTestCase(test.NoDBTestCase, drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) dev_name = "net_enp2s2_02_9a_a1_37_be_54" parent_address = "pci_0000_04_11_7" - node_dev = FakeNodeDevice(_fake_NodeDevXml[dev_name]) - - with mock.patch.object(pci_utils, 'get_net_name_by_vf_pci_address', - return_value=dev_name) as mock_get_net_name, \ - mock.patch.object(drvr._host, 'device_lookup_by_name', - return_value=node_dev) as mock_dev_lookup: - actualvf = drvr._get_pcinet_info(parent_address) - expect_vf = { - "name": dev_name, - "capabilities": ["rx", "tx", "sg", "tso", "gso", "gro", - "rxvlan", "txvlan"] - } - self.assertEqual(expect_vf, actualvf) - mock_get_net_name.assert_called_once_with(parent_address) - mock_dev_lookup.assert_called_once_with(dev_name) - - def test_get_pcinet_info_raises(self): - drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) - dev_name = "net_enp2s2_02_9a_a1_37_be_54" - parent_address = "pci_0000_04_11_7" - - with mock.patch.object(pci_utils, 'get_net_name_by_vf_pci_address', - return_value=dev_name) as mock_get_net_name, \ - mock.patch.object( - drvr._host, 'device_lookup_by_name', - side_effect=fakelibvirt.libvirtError("message") - ) as mock_dev_lookup: - actualvf = drvr._get_pcinet_info(parent_address) - self.assertIsNone(actualvf) - mock_get_net_name.assert_called_once_with(parent_address) - mock_dev_lookup.assert_called_once_with(dev_name) + net_dev = fakelibvirt.NodeDevice( + drvr._get_connection(), xml=_fake_NodeDevXml[dev_name]) + pci_dev = fakelibvirt.NodeDevice( + drvr._get_connection(), xml=_fake_NodeDevXml[parent_address]) + actualvf = drvr._get_pcinet_info(pci_dev, [net_dev]) + expect_vf = ["rx", "tx", "sg", "tso", "gso", "gro", "rxvlan", "txvlan"] + self.assertEqual(expect_vf, actualvf) @mock.patch.object(pci_utils, 'get_ifname_by_pci_address') def test_get_pcidev_info_non_nic(self, mock_get_ifname): - self.stub_out('nova.virt.libvirt.host.Host.device_lookup_by_name', - lambda self, name: FakeNodeDevice( - _fake_NodeDevXml[name])) - drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) - id = "pci_0000_04_10_7" - mock_get_ifname.side_effect = exception.PciDeviceNotFoundById(id=id) - actualvf = drvr._get_pcidev_info(id) + dev_name = "pci_0000_04_11_7" + pci_dev = fakelibvirt.NodeDevice( + drvr._get_connection(), xml=_fake_NodeDevXml[dev_name]) + actual_vf = drvr._get_pcidev_info(dev_name, pci_dev, []) expect_vf = { - "dev_id": id, - "address": "0000:04:10.7", - "product_id": '1520', - "numa_node": None, - "vendor_id": '8086', - "label": 'label_8086_1520', + "dev_id": dev_name, "address": "0000:04:11.7", + "product_id": '1520', "numa_node": 0, + "vendor_id": '8086', "label": 'label_8086_1520', "dev_type": fields.PciDeviceType.SRIOV_VF, 'parent_addr': '0000:04:00.3', - } - self.assertEqual(expect_vf, actualvf) + } + self.assertEqual(expect_vf, actual_vf) + mock_get_ifname.assert_not_called() @mock.patch.object(pci_utils, 'get_ifname_by_pci_address', return_value='ens1') def test_get_pcidev_info(self, mock_get_ifname): - self.stub_out('nova.virt.libvirt.host.Host.device_lookup_by_name', - lambda self, name: FakeNodeDevice( - _fake_NodeDevXml[name])) - drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) - actualvf = drvr._get_pcidev_info("pci_0000_04_00_3") + devs = { + "pci_0000_04_00_3", "pci_0000_04_10_7", "pci_0000_04_11_7", + "pci_0000_04_00_1", "pci_0000_03_00_0", "pci_0000_03_00_1" + } + node_devs = {} + for dev_name in devs: + node_devs[dev_name] = ( + fakelibvirt.NodeDevice( + drvr._get_connection(), xml=_fake_NodeDevXml[dev_name])) + for child in _fake_NodeDevXml_children[dev_name]: + node_devs[child] = ( + fakelibvirt.NodeDevice( + drvr._get_connection(), + xml=_fake_NodeDevXml[child])) + net_devs = [ + dev for dev in node_devs.values() if dev.name() not in devs] + + name = "pci_0000_04_00_3" + actual_vf = drvr._get_pcidev_info(name, node_devs[name], net_devs) expect_vf = { "dev_id": "pci_0000_04_00_3", "address": "0000:04:00.3", @@ -17298,9 +17312,10 @@ class LibvirtConnTestCase(test.NoDBTestCase, "label": 'label_8086_1521', "dev_type": fields.PciDeviceType.SRIOV_PF, } - self.assertEqual(expect_vf, actualvf) + self.assertEqual(expect_vf, actual_vf) - actualvf = drvr._get_pcidev_info("pci_0000_04_10_7") + name = "pci_0000_04_10_7" + actual_vf = drvr._get_pcidev_info(name, node_devs[name], net_devs) expect_vf = { "dev_id": "pci_0000_04_10_7", "address": "0000:04:10.7", @@ -17311,29 +17326,32 @@ class LibvirtConnTestCase(test.NoDBTestCase, "dev_type": fields.PciDeviceType.SRIOV_VF, "parent_addr": '0000:04:00.3', "parent_ifname": "ens1", + "capabilities": { + "network": ["rx", "tx", "sg", "tso", "gso", "gro", + "rxvlan", "txvlan"]}, } - self.assertEqual(expect_vf, actualvf) + self.assertEqual(expect_vf, actual_vf) - with mock.patch.object(pci_utils, 'get_net_name_by_vf_pci_address', - return_value="net_enp2s2_02_9a_a1_37_be_54"): - actualvf = drvr._get_pcidev_info("pci_0000_04_11_7") - expect_vf = { - "dev_id": "pci_0000_04_11_7", - "address": "0000:04:11.7", - "product_id": '1520', - "vendor_id": '8086', - "numa_node": 0, - "label": 'label_8086_1520', - "dev_type": fields.PciDeviceType.SRIOV_VF, - "parent_addr": '0000:04:00.3', - "capabilities": { - "network": ["rx", "tx", "sg", "tso", "gso", "gro", - "rxvlan", "txvlan"]}, - "parent_ifname": "ens1", - } - self.assertEqual(expect_vf, actualvf) + name = "pci_0000_04_11_7" + actual_vf = drvr._get_pcidev_info(name, node_devs[name], net_devs) + expect_vf = { + "dev_id": "pci_0000_04_11_7", + "address": "0000:04:11.7", + "product_id": '1520', + "vendor_id": '8086', + "numa_node": 0, + "label": 'label_8086_1520', + "dev_type": fields.PciDeviceType.SRIOV_VF, + "parent_addr": '0000:04:00.3', + "capabilities": { + "network": ["rx", "tx", "sg", "tso", "gso", "gro", + "rxvlan", "txvlan"]}, + "parent_ifname": "ens1", + } + self.assertEqual(expect_vf, actual_vf) - actualvf = drvr._get_pcidev_info("pci_0000_04_00_1") + name = "pci_0000_04_00_1" + actual_vf = drvr._get_pcidev_info(name, node_devs[name], net_devs) expect_vf = { "dev_id": "pci_0000_04_00_1", "address": "0000:04:00.1", @@ -17343,9 +17361,10 @@ class LibvirtConnTestCase(test.NoDBTestCase, "label": 'label_15b3_1013', "dev_type": fields.PciDeviceType.STANDARD, } - self.assertEqual(expect_vf, actualvf) + self.assertEqual(expect_vf, actual_vf) - actualvf = drvr._get_pcidev_info("pci_0000_03_00_0") + name = "pci_0000_03_00_0" + actual_vf = drvr._get_pcidev_info(name, node_devs[name], net_devs) expect_vf = { "dev_id": "pci_0000_03_00_0", "address": "0000:03:00.0", @@ -17355,9 +17374,10 @@ class LibvirtConnTestCase(test.NoDBTestCase, "label": 'label_15b3_1013', "dev_type": fields.PciDeviceType.SRIOV_PF, } - self.assertEqual(expect_vf, actualvf) + self.assertEqual(expect_vf, actual_vf) - actualvf = drvr._get_pcidev_info("pci_0000_03_00_1") + name = "pci_0000_03_00_1" + actual_vf = drvr._get_pcidev_info(name, node_devs[name], net_devs) expect_vf = { "dev_id": "pci_0000_03_00_1", "address": "0000:03:00.1", @@ -17367,36 +17387,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, "label": 'label_15b3_1013', "dev_type": fields.PciDeviceType.SRIOV_PF, } - self.assertEqual(expect_vf, actualvf) - - def test_list_devices_not_supported(self): - drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) - - # Handle just the NO_SUPPORT error - not_supported_exc = fakelibvirt.make_libvirtError( - fakelibvirt.libvirtError, - 'this function is not supported by the connection driver:' - ' virNodeNumOfDevices', - error_code=fakelibvirt.VIR_ERR_NO_SUPPORT) - - with mock.patch.object(host.Host, '_list_devices', - side_effect=not_supported_exc): - self.assertEqual('[]', drvr._get_pci_passthrough_devices()) - - # We cache not supported status to avoid emitting too many logging - # messages. Clear this value to test the other exception case. - del drvr._list_devices_supported - - # Other errors should not be caught - other_exc = fakelibvirt.make_libvirtError( - fakelibvirt.libvirtError, - 'other exc', - error_code=fakelibvirt.VIR_ERR_NO_DOMAIN) - - with mock.patch.object(host.Host, '_list_devices', - side_effect=other_exc): - self.assertRaises(fakelibvirt.libvirtError, - drvr._get_pci_passthrough_devices) + self.assertEqual(expect_vf, actual_vf) @mock.patch.object(pci_utils, 'get_ifname_by_pci_address', return_value='ens1') @@ -17404,13 +17395,31 @@ class LibvirtConnTestCase(test.NoDBTestCase, return_value=['pci_0000_04_00_3', 'pci_0000_04_10_7', 'pci_0000_04_11_7']) def test_get_pci_passthrough_devices(self, mock_list, mock_get_ifname): - self.stub_out('nova.virt.libvirt.host.Host.device_lookup_by_name', - lambda self, name: FakeNodeDevice( - _fake_NodeDevXml[name])) drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) - actjson = drvr._get_pci_passthrough_devices() + devs = ['pci_0000_04_00_3', 'pci_0000_04_10_7', 'pci_0000_04_11_7'] + node_devs = {} + for dev_name in devs: + node_devs[dev_name] = ( + fakelibvirt.NodeDevice( + drvr._get_connection(), xml=_fake_NodeDevXml[dev_name])) + for child in _fake_NodeDevXml_children[dev_name]: + node_devs[child] = ( + fakelibvirt.NodeDevice( + drvr._get_connection(), + xml=_fake_NodeDevXml[child])) + + self.useFixture(fixtures.MockPatchObject( + drvr._host, 'list_all_devices', + return_value=node_devs.values())) + + self.stub_out( + 'nova.virt.libvirt.host.Host.device_lookup_by_name', + lambda self, name: node_devs.get(name)) + + actjson = drvr._get_pci_passthrough_devices() + mock_list.assert_not_called() expectvfs = [ { "dev_id": "pci_0000_04_00_3", @@ -17449,17 +17458,15 @@ class LibvirtConnTestCase(test.NoDBTestCase, actualvfs = jsonutils.loads(actjson) for dev in range(len(actualvfs)): for key in actualvfs[dev].keys(): - if key not in ['phys_function', 'virt_functions', 'label']: + if key not in ['phys_function', 'virt_functions', + 'label', 'capabilities']: self.assertEqual(expectvfs[dev][key], actualvfs[dev][key]) - mock_list.assert_called_once_with() # The first call for every VF is to determine parent_ifname and # the second call to determine the MAC address. mock_get_ifname.assert_has_calls([ mock.call('0000:04:10.7', pf_interface=True), - mock.call('0000:04:10.7', False), mock.call('0000:04:11.7', pf_interface=True), - mock.call('0000:04:11.7', False) ]) @mock.patch.object(host.Host, 'has_min_version', diff --git a/nova/tests/unit/virt/libvirt/test_fakelibvirt.py b/nova/tests/unit/virt/libvirt/test_fakelibvirt.py index 186a1cddaaf9..09c54bc791f3 100644 --- a/nova/tests/unit/virt/libvirt/test_fakelibvirt.py +++ b/nova/tests/unit/virt/libvirt/test_fakelibvirt.py @@ -457,7 +457,7 @@ class FakeLibvirtTests(test.NoDBTestCase): vf_xml = """ pci_0000_81_00_1 /sys/devices/pci0000:80/0000:80:01.0/0000:81:00.1 - pci_0000_80_01_0 + pci_0000_81_00_0 ixgbevf diff --git a/nova/tests/unit/virt/libvirt/test_host.py b/nova/tests/unit/virt/libvirt/test_host.py index 8409159f7b1c..c000f480c0e6 100644 --- a/nova/tests/unit/virt/libvirt/test_host.py +++ b/nova/tests/unit/virt/libvirt/test_host.py @@ -1136,6 +1136,74 @@ Active: 8381604 kB self.host.list_mediated_devices(8) mock_listDevices.assert_called_once_with('mdev', flags=8) + def test_list_all_devices(self): + with mock.patch.object( + self.host.get_connection(), + "listAllDevices") as mock_list_all_devices: + xml_str = """ + + pci_0000_04_00_3 + pci_0000_00_01_1 + + igb + + + 0 + 4 + 0 + 3 + I350 Gigabit Network Connection + Intel Corporation + +
+
+
+
+ + + """ + pci_dev = fakelibvirt.NodeDevice(None, xml=xml_str) + node_devs = [pci_dev] + mock_list_all_devices.return_value = node_devs + ret = self.host.list_all_devices(flags=42) + self.assertEqual(node_devs, ret) + mock_list_all_devices.assert_called_once_with(42) + + def test_list_all_devices_raises(self): + with mock.patch.object( + self.host.get_connection(), + "listAllDevices") as mock_list_all_devices: + xml_str = """ + + pci_0000_04_00_3 + pci_0000_00_01_1 + + igb + + + 0 + 4 + 0 + 3 + I350 Gigabit Network Connection + Intel Corporation + +
+
+
+
+ + + """ + pci_dev = fakelibvirt.NodeDevice(None, xml=xml_str) + node_devs = [pci_dev] + mock_list_all_devices.return_value = node_devs + mock_list_all_devices.side_effect = fakelibvirt.libvirtError( + "message") + ret = self.host.list_all_devices(flags=42) + self.assertEqual([], ret) + mock_list_all_devices.assert_called_once_with(42) + @mock.patch.object(fakelibvirt.virConnect, "listDevices") def test_list_devices(self, mock_listDevices): self.host._list_devices('mdev', 8) diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index e2ee16df7ef6..5443e55472da 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -6908,35 +6908,41 @@ class LibvirtDriver(driver.ComputeDriver): cpu_info['features'] = features return cpu_info - def _get_pcinet_info(self, vf_address): + def _get_pcinet_info( + self, + dev: 'libvirt.virNodeDevice', + net_devs: ty.List['libvirt.virNodeDevice'] + ) -> ty.Optional[ty.List[str]]: """Returns a dict of NET device.""" - devname = pci_utils.get_net_name_by_vf_pci_address(vf_address) - if not devname: + net_dev = {dev.parent(): dev for dev in net_devs}.get(dev.name(), None) + if net_dev is None: return None - - try: - virtdev = self._host.device_lookup_by_name(devname) - except libvirt.libvirtError as ex: - LOG.warning(ex) - return None - xmlstr = virtdev.XMLDesc(0) + xmlstr = net_dev.XMLDesc(0) cfgdev = vconfig.LibvirtConfigNodeDevice() cfgdev.parse_str(xmlstr) - return {'name': cfgdev.name, - 'capabilities': cfgdev.pci_capability.features} + return cfgdev.pci_capability.features - def _get_pcidev_info(self, devname): + def _get_pcidev_info( + self, + devname: str, + dev: 'libvirt.virNodeDevice', + net_devs: ty.List['libvirt.virNodeDevice'] + ) -> ty.Dict[str, ty.Union[str, dict]]: """Returns a dict of PCI device.""" - def _get_device_type(cfgdev, pci_address): + def _get_device_type( + cfgdev: vconfig.LibvirtConfigNodeDevice, + pci_address: str, + device: 'libvirt.virNodeDevice', + net_devs: ty.List['libvirt.virNodeDevice'] + ) -> ty.Dict[str, str]: """Get a PCI device's device type. An assignable PCI device can be a normal PCI device, a SR-IOV Physical Function (PF), or a SR-IOV Virtual - Function (VF). Only normal PCI devices or SR-IOV VFs - are assignable, while SR-IOV PFs are always owned by - hypervisor. + Function (VF). """ + net_dev_parents = {dev.parent() for dev in net_devs} for fun_cap in cfgdev.pci_capability.fun_capability: if fun_cap.type == 'virt_functions': return { @@ -6954,35 +6960,34 @@ class LibvirtDriver(driver.ComputeDriver): 'parent_addr': phys_address, } parent_ifname = None - try: + # NOTE(sean-k-mooney): if the VF is a parent of a netdev + # the PF should also have a netdev. + if device.name() in net_dev_parents: parent_ifname = pci_utils.get_ifname_by_pci_address( pci_address, pf_interface=True) - except exception.PciDeviceNotFoundById: - # NOTE(sean-k-mooney): we ignore this error as it - # is expected when the virtual function is not a NIC. - pass - if parent_ifname: result['parent_ifname'] = parent_ifname return result return {'dev_type': fields.PciDeviceType.STANDARD} - def _get_device_capabilities(device, address): + def _get_device_capabilities( + device_dict: dict, + device: 'libvirt.virNodeDevice', + net_devs: ty.List['libvirt.virNodeDevice'] + ) -> ty.Dict[str, ty.Dict[str, ty.Optional[ty.List[str]]]]: """Get PCI VF device's additional capabilities. If a PCI device is a virtual function, this function reads the PCI parent's network capabilities (must be always a NIC device) and appends this information to the device's dictionary. """ - if device.get('dev_type') == fields.PciDeviceType.SRIOV_VF: - pcinet_info = self._get_pcinet_info(address) + if device_dict.get('dev_type') == fields.PciDeviceType.SRIOV_VF: + pcinet_info = self._get_pcinet_info(device, net_devs) if pcinet_info: - return {'capabilities': - {'network': pcinet_info.get('capabilities')}} + return {'capabilities': {'network': pcinet_info}} return {} - virtdev = self._host.device_lookup_by_name(devname) - xmlstr = virtdev.XMLDesc(0) + xmlstr = dev.XMLDesc(0) cfgdev = vconfig.LibvirtConfigNodeDevice() cfgdev.parse_str(xmlstr) @@ -7003,8 +7008,8 @@ class LibvirtDriver(driver.ComputeDriver): # requirement by DataBase Model device['label'] = 'label_%(vendor_id)s_%(product_id)s' % device - device.update(_get_device_type(cfgdev, address)) - device.update(_get_device_capabilities(device, address)) + device.update(_get_device_type(cfgdev, address, dev, net_devs)) + device.update(_get_device_capabilities(device, dev, net_devs)) return device def _get_pci_passthrough_devices(self): @@ -7022,28 +7027,13 @@ class LibvirtDriver(driver.ComputeDriver): :returns: a JSON string containing a list of the assignable PCI devices information """ - # Bail early if we know we can't support `listDevices` to avoid - # repeated warnings within a periodic task - if not getattr(self, '_list_devices_supported', True): - return jsonutils.dumps([]) - - try: - dev_names = self._host.list_pci_devices() or [] - except libvirt.libvirtError as ex: - error_code = ex.get_error_code() - if error_code == libvirt.VIR_ERR_NO_SUPPORT: - self._list_devices_supported = False - LOG.warning("URI %(uri)s does not support " - "listDevices: %(error)s", - {'uri': self._uri(), - 'error': encodeutils.exception_to_unicode(ex)}) - return jsonutils.dumps([]) - else: - raise - - pci_info = [] - for name in dev_names: - pci_info.append(self._get_pcidev_info(name)) + dev_flags = (libvirt.VIR_CONNECT_LIST_NODE_DEVICES_CAP_NET | + libvirt.VIR_CONNECT_LIST_NODE_DEVICES_CAP_PCI_DEV) + devices = {dev.name(): dev for dev in + self._host.list_all_devices(flags=dev_flags)} + net_devs = [dev for dev in devices.values() if "net" in dev.listCaps()] + pci_info = [self._get_pcidev_info(name, dev, net_devs) for name, dev + in devices.items() if "pci" in dev.listCaps()] return jsonutils.dumps(pci_info) diff --git a/nova/virt/libvirt/host.py b/nova/virt/libvirt/host.py index 4bf07e7734f3..26a2d0970e07 100644 --- a/nova/virt/libvirt/host.py +++ b/nova/virt/libvirt/host.py @@ -31,8 +31,10 @@ from collections import defaultdict import inspect import operator import os +import queue import socket import threading +import typing as ty from eventlet import greenio from eventlet import greenthread @@ -59,7 +61,10 @@ from nova.virt.libvirt import guest as libvirt_guest from nova.virt.libvirt import migration as libvirt_migrate from nova.virt.libvirt import utils as libvirt_utils -libvirt = None +if ty.TYPE_CHECKING: + import libvirt +else: + libvirt = None LOG = logging.getLogger(__name__) @@ -92,7 +97,8 @@ class Host(object): self._read_only = read_only self._initial_connection = True self._conn_event_handler = conn_event_handler - self._conn_event_handler_queue = six.moves.queue.Queue() + self._conn_event_handler_queue: queue.Queue[ty.Callable] = ( + queue.Queue()) self._lifecycle_event_handler = lifecycle_event_handler self._caps = None self._domain_caps = None @@ -100,7 +106,7 @@ class Host(object): self._wrapped_conn = None self._wrapped_conn_lock = threading.Lock() - self._event_queue = None + self._event_queue: ty.Optional[queue.Queue[ty.Callable]] = None self._events_delayed = {} # Note(toabctl): During a reboot of a domain, STOPPED and @@ -320,9 +326,14 @@ class Host(object): # Process as many events as possible without # blocking last_close_event = None + # required for mypy + if self._event_queue is None: + return while not self._event_queue.empty(): try: - event = self._event_queue.get(block=False) + event_type = ty.Union[ + virtevent.LifecycleEvent, ty.Mapping[str, ty.Any]] + event: event_type = self._event_queue.get(block=False) if isinstance(event, virtevent.LifecycleEvent): # call possibly with delay self._event_emit_delayed(event) @@ -809,7 +820,7 @@ class Host(object): if self._domain_caps: return self._domain_caps - domain_caps = defaultdict(dict) + domain_caps: ty.Dict = defaultdict(dict) caps = self.get_capabilities() virt_type = CONF.libvirt.virt_type @@ -1183,6 +1194,19 @@ class Host(object): else: raise + def list_all_devices( + self, flags: int = 0) -> ty.List['libvirt.virNodeDevice']: + """Lookup devices. + + :param flags: a bitmask of flags to filter the returned devices. + :returns: a list of virNodeDevice xml strings. + """ + try: + return self.get_connection().listAllDevices(flags) or [] + except libvirt.libvirtError as ex: + LOG.warning(ex) + return [] + def compare_cpu(self, xmlDesc, flags=0): """Compares the given CPU description with the host CPU.""" return self.get_connection().compareCPU(xmlDesc, flags) diff --git a/releasenotes/notes/libvirt-nodedev-lookup-d80174ac30bc82f0.yaml b/releasenotes/notes/libvirt-nodedev-lookup-d80174ac30bc82f0.yaml new file mode 100644 index 000000000000..5f0d7e6267d5 --- /dev/null +++ b/releasenotes/notes/libvirt-nodedev-lookup-d80174ac30bc82f0.yaml @@ -0,0 +1,12 @@ +--- +fixes: + - | + Since the 16.0.0 (Pike) release, nova has collected NIC feature + flags via libvirt. To look up the NIC feature flags for a whitelisted + PCI device the nova libvirt driver computed the libvirt nodedev name + by rendering a format string using the netdev name associated with + the interface and its current MAC address. In some environments the + libvirt nodedev list can become out of sync with the current MAC address + assigned to a netdev and as a result the nodedev look up can fail. + Nova now uses PCI addresses, rather than MAC addresses, to look up these + PCI network devices.