diff --git a/nova/pci/manager.py b/nova/pci/manager.py
index 3084643f5e8a..05930b0bebb0 100644
--- a/nova/pci/manager.py
+++ b/nova/pci/manager.py
@@ -225,6 +225,7 @@ class PciDevTracker(object):
self.stale[new_value['address']] = new_value
else:
existed.update_device(new_value)
+ self.stats.update_device(existed)
# Track newly discovered devices.
for dev in [dev for dev in devices if
diff --git a/nova/pci/stats.py b/nova/pci/stats.py
index b911410848d0..232e94ff6f0b 100644
--- a/nova/pci/stats.py
+++ b/nova/pci/stats.py
@@ -104,6 +104,32 @@ class PciDeviceStats(object):
pool['parent_ifname'] = dev.extra_info['parent_ifname']
return pool
+ def _get_pool_with_device_type_mismatch(self, dev):
+ """Check for device type mismatch in the pools for a given device.
+
+ Return (pool, device) if device type does not match or a single None
+ if the device type matches.
+ """
+ for pool in self.pools:
+ for device in pool['devices']:
+ if device.address == dev.address:
+ if dev.dev_type != pool["dev_type"]:
+ return pool, device
+ return None
+
+ return None
+
+ def update_device(self, dev):
+ """Update a device to its matching pool."""
+ pool_device_info = self._get_pool_with_device_type_mismatch(dev)
+ if pool_device_info is None:
+ return
+
+ pool, device = pool_device_info
+ pool['devices'].remove(device)
+ self._decrease_pool_count(self.pools, pool)
+ self.add_device(dev)
+
def add_device(self, dev):
"""Add a device to its matching pool."""
dev_pool = self._create_pool_keys_from_dev(dev)
diff --git a/nova/tests/functional/libvirt/test_pci_sriov_servers.py b/nova/tests/functional/libvirt/test_pci_sriov_servers.py
index d494d66a07c6..0d9463165af3 100644
--- a/nova/tests/functional/libvirt/test_pci_sriov_servers.py
+++ b/nova/tests/functional/libvirt/test_pci_sriov_servers.py
@@ -70,6 +70,7 @@ class SRIOVServersTest(_PCIServersTestBase):
{
'vendor_id': fakelibvirt.PCI_VEND_ID,
'product_id': fakelibvirt.PF_PROD_ID,
+ 'physical_network': 'physnet4',
},
{
'vendor_id': fakelibvirt.PCI_VEND_ID,
@@ -103,6 +104,20 @@ class SRIOVServersTest(_PCIServersTestBase):
# fixture already stubbed.
self.neutron = self.useFixture(base.LibvirtNeutronFixture(self))
+ def _disable_sriov_in_pf(self, pci_info):
+ # Check for PF and change the capability from virt_functions
+ # Delete all the VFs
+ vfs_to_delete = []
+
+ for device_name, device in pci_info.devices.items():
+ if 'virt_functions' in device.pci_device:
+ device.generate_xml(skip_capability=True)
+ elif 'phys_function' in device.pci_device:
+ vfs_to_delete.append(device_name)
+
+ for device in vfs_to_delete:
+ del pci_info.devices[device]
+
def test_create_server_with_VF(self):
"""Create a server with an SR-IOV VF-type PCI device."""
@@ -266,6 +281,69 @@ class SRIOVServersTest(_PCIServersTestBase):
)
self.assertIsNone(diagnostics['nic_details'][1]['tx_packets'])
+ def test_create_server_after_change_in_nonsriov_pf_to_sriov_pf(self):
+ # Starts a compute with PF not configured with SRIOV capabilities
+ # Updates the PF with SRIOV capability and restart the compute service
+ # Then starts a VM with the sriov port. The VM should be in active
+ # state with sriov port attached.
+
+ # To emulate the device type changing, we first create a
+ # HostPCIDevicesInfo object with PFs and VFs. Then we make a copy
+ # and remove the VFs and the virt_function capability. This is
+ # done to ensure the physical function product id is same in both
+ # the versions.
+ pci_info = fakelibvirt.HostPCIDevicesInfo(num_pfs=1, num_vfs=1)
+ pci_info_no_sriov = copy.deepcopy(pci_info)
+
+ # Disable SRIOV capabilties in PF and delete the VFs
+ self._disable_sriov_in_pf(pci_info_no_sriov)
+
+ fake_connection = self._get_connection(pci_info=pci_info_no_sriov,
+ hostname='test_compute0')
+ self.mock_conn.return_value = fake_connection
+
+ self.compute = self.start_service('compute', host='test_compute0')
+
+ ctxt = context.get_admin_context()
+ pci_devices = objects.PciDeviceList.get_by_compute_node(
+ ctxt,
+ objects.ComputeNode.get_by_nodename(
+ ctxt, 'test_compute0',
+ ).id,
+ )
+ self.assertEqual(1, len(pci_devices))
+ self.assertEqual('type-PCI', pci_devices[0].dev_type)
+
+ # Update connection with original pci info with sriov PFs
+ fake_connection = self._get_connection(pci_info=pci_info,
+ hostname='test_compute0')
+ self.mock_conn.return_value = fake_connection
+
+ # Restart the compute service
+ self.restart_compute_service(self.compute)
+
+ # Verify if PCI devices are of type type-PF or type-VF
+ pci_devices = objects.PciDeviceList.get_by_compute_node(
+ ctxt,
+ objects.ComputeNode.get_by_nodename(
+ ctxt, 'test_compute0',
+ ).id,
+ )
+ for pci_device in pci_devices:
+ self.assertIn(pci_device.dev_type, ['type-PF', 'type-VF'])
+
+ # create the port
+ self.neutron.create_port({'port': self.neutron.network_4_port_1})
+
+ # create a server using the VF via neutron
+ flavor_id = self._create_flavor()
+ self._create_server(
+ flavor_id=flavor_id,
+ networks=[
+ {'port': base.LibvirtNeutronFixture.network_4_port_1['id']},
+ ],
+ )
+
class SRIOVAttachDetachTest(_PCIServersTestBase):
# no need for aliases as these test will request SRIOV via neutron
diff --git a/nova/tests/unit/pci/test_stats.py b/nova/tests/unit/pci/test_stats.py
index 1671e9f000f8..6c1e55f8cded 100644
--- a/nova/tests/unit/pci/test_stats.py
+++ b/nova/tests/unit/pci/test_stats.py
@@ -561,6 +561,30 @@ class PciDeviceStatsWithTagsTestCase(test.NoDBTestCase):
self.pci_stats.remove_device(dev2)
self._assertPools()
+ def test_update_device(self):
+ # Update device type of one of the device from type-PCI to
+ # type-PF. Verify if the existing pool is updated and a new
+ # pool is created with dev_type type-PF.
+ self._create_pci_devices()
+ dev1 = self.pci_tagged_devices.pop()
+ dev1.dev_type = 'type-PF'
+ self.pci_stats.update_device(dev1)
+ self.assertEqual(3, len(self.pci_stats.pools))
+ self._assertPoolContent(self.pci_stats.pools[0], '1137', '0072',
+ len(self.pci_untagged_devices))
+ self.assertEqual(self.pci_untagged_devices,
+ self.pci_stats.pools[0]['devices'])
+ self._assertPoolContent(self.pci_stats.pools[1], '1137', '0071',
+ len(self.pci_tagged_devices),
+ physical_network='physnet1')
+ self.assertEqual(self.pci_tagged_devices,
+ self.pci_stats.pools[1]['devices'])
+ self._assertPoolContent(self.pci_stats.pools[2], '1137', '0071',
+ 1,
+ physical_network='physnet1')
+ self.assertEqual(dev1,
+ self.pci_stats.pools[2]['devices'][0])
+
class PciDeviceVFPFStatsTestCase(test.NoDBTestCase):
diff --git a/nova/tests/unit/virt/libvirt/fakelibvirt.py b/nova/tests/unit/virt/libvirt/fakelibvirt.py
index 2de1eece22c6..2abda6b657fa 100644
--- a/nova/tests/unit/virt/libvirt/fakelibvirt.py
+++ b/nova/tests/unit/virt/libvirt/fakelibvirt.py
@@ -284,49 +284,62 @@ class FakePCIDevice(object):
:param multiple_gpu_types: (bool) Supports different vGPU types
"""
+ self.dev_type = dev_type
+ self.slot = slot
+ self.function = function
+ self.iommu_group = iommu_group
+ self.numa_node = numa_node
+ self.vf_ratio = vf_ratio
+ self.multiple_gpu_types = multiple_gpu_types
+ self.parent = parent
+ self.generate_xml()
+
+ def generate_xml(self, skip_capability=False):
vend_id = PCI_VEND_ID
vend_name = PCI_VEND_NAME
- if dev_type == 'PCI':
- if vf_ratio:
+ capability = ''
+ if self.dev_type == 'PCI':
+ if self.vf_ratio:
raise ValueError('vf_ratio does not apply for PCI devices')
prod_id = PCI_PROD_ID
prod_name = PCI_PROD_NAME
driver = PCI_DRIVER_NAME
- capability = ''
- elif dev_type == 'PF':
+ elif self.dev_type == 'PF':
prod_id = PF_PROD_ID
prod_name = PF_PROD_NAME
driver = PF_DRIVER_NAME
- capability = self.cap_templ % {
- 'cap_type': PF_CAP_TYPE,
- 'addresses': '\n'.join([
- self.addr_templ % {
- # these are the slot, function values of the child VFs
- # we can only assign 8 functions to a slot (0-7) so
- # bump the slot each time we exceed this
- 'slot': slot + (x // 8),
- # ...and wrap the function value
- 'function': x % 8,
- # the offset is because the PF is occupying function 0
- } for x in range(1, vf_ratio + 1)])
- }
- elif dev_type == 'VF':
+ if not skip_capability:
+ capability = self.cap_templ % {
+ 'cap_type': PF_CAP_TYPE,
+ 'addresses': '\n'.join([
+ self.addr_templ % {
+ # these are the slot, function values of the child
+ # VFs, we can only assign 8 functions to a slot
+ # (0-7) so bump the slot each time we exceed this
+ 'slot': self.slot + (x // 8),
+ # ...and wrap the function value
+ 'function': x % 8,
+ # the offset is because the PF is occupying function 0
+ } for x in range(1, self.vf_ratio + 1)])
+ }
+ elif self.dev_type == 'VF':
prod_id = VF_PROD_ID
prod_name = VF_PROD_NAME
driver = VF_DRIVER_NAME
- capability = self.cap_templ % {
- 'cap_type': VF_CAP_TYPE,
- 'addresses': self.addr_templ % {
- # this is the slot, function value of the parent PF
- # if we're e.g. device 8, we'll have a different slot
- # to our parent so reverse this
- 'slot': slot - ((vf_ratio + 1) // 8),
- # the parent PF is always function 0
- 'function': 0,
+ if not skip_capability:
+ capability = self.cap_templ % {
+ 'cap_type': VF_CAP_TYPE,
+ 'addresses': self.addr_templ % {
+ # this is the slot, function value of the parent PF
+ # if we're e.g. device 8, we'll have a different slot
+ # to our parent so reverse this
+ 'slot': self.slot - ((self.vf_ratio + 1) // 8),
+ # the parent PF is always function 0
+ 'function': 0,
+ }
}
- }
- elif dev_type == 'MDEV_TYPES':
+ elif self.dev_type == 'MDEV_TYPES':
prod_id = MDEV_CAPABLE_PROD_ID
prod_name = MDEV_CAPABLE_PROD_NAME
driver = MDEV_CAPABLE_DRIVER_NAME
@@ -336,36 +349,37 @@ class FakePCIDevice(object):
'type_id': NVIDIA_11_VGPU_TYPE,
'instances': 16,
}]
- if multiple_gpu_types:
+ if self.multiple_gpu_types:
types.append(self.mdevtypes_templ % {
'type_id': NVIDIA_12_VGPU_TYPE,
'instances': 8,
})
- capability = self.cap_templ % {
- 'cap_type': MDEV_CAPABLE_CAP_TYPE,
- 'addresses': '\n'.join(types)
- }
+ if not skip_capability:
+ capability = self.cap_templ % {
+ 'cap_type': MDEV_CAPABLE_CAP_TYPE,
+ 'addresses': '\n'.join(types)
+ }
self.is_capable_of_mdevs = True
else:
raise ValueError('Expected one of: PCI, VF, PCI')
self.pci_device = self.pci_device_template % {
- 'slot': slot,
- 'function': function,
+ 'slot': self.slot,
+ 'function': self.function,
'vend_id': vend_id,
'vend_name': vend_name,
'prod_id': prod_id,
'prod_name': prod_name,
'driver': driver,
'capability': capability,
- 'iommu_group': iommu_group,
- 'numa_node': numa_node,
- 'parent': parent or self.pci_default_parent
+ 'iommu_group': self.iommu_group,
+ 'numa_node': self.numa_node,
+ 'parent': self.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
# Libvirt omits the NUMA element so we remove it.
- if numa_node == -1:
+ if self.numa_node == -1:
self.pci_device = self.pci_device.replace("", "")
def XMLDesc(self, flags):
@@ -943,6 +957,20 @@ class Domain(object):
nic_info['source'] = source.get('network')
elif nic_info['type'] == 'bridge':
nic_info['source'] = source.get('bridge')
+ elif nic_info['type'] == 'hostdev':
+ # is for VF when vnic_type
+ # is direct. Add sriov vf pci information in nic_info
+ address = source.find('./address')
+ pci_type = address.get('type')
+ pci_domain = address.get('domain').replace('0x', '')
+ pci_bus = address.get('bus').replace('0x', '')
+ pci_slot = address.get('slot').replace('0x', '')
+ pci_function = address.get('function').replace(
+ '0x', '')
+ pci_device = "%s_%s_%s_%s_%s" % (pci_type, pci_domain,
+ pci_bus, pci_slot,
+ pci_function)
+ nic_info['source'] = pci_device
nics_info += [nic_info]
@@ -984,11 +1012,32 @@ class Domain(object):
return definition
+ def verify_hostdevs_interface_are_vfs(self):
+ """Verify for interface type hostdev if the pci device is VF or not.
+ """
+
+ error_message = ("Interface type hostdev is currently supported on "
+ "SR-IOV Virtual Functions only")
+
+ nics = self._def['devices'].get('nics', [])
+ for nic in nics:
+ if nic['type'] == 'hostdev':
+ pci_device = nic['source']
+ pci_info_from_connection = self._connection.pci_info.devices[
+ pci_device]
+ if 'phys_function' not in pci_info_from_connection.pci_device:
+ raise make_libvirtError(
+ libvirtError,
+ error_message,
+ error_code=VIR_ERR_CONFIG_UNSUPPORTED,
+ error_domain=VIR_FROM_DOMAIN)
+
def create(self):
self.createWithFlags(0)
def createWithFlags(self, flags):
# FIXME: Not handling flags at the moment
+ self.verify_hostdevs_interface_are_vfs()
self._state = VIR_DOMAIN_RUNNING
self._connection._mark_running(self)
self._has_saved_state = False
@@ -1112,7 +1161,7 @@ class Domain(object):
nics = ''
for nic in self._def['devices']['nics']:
- if 'source' in nic:
+ if 'source' in nic and nic['type'] != 'hostdev':
nics += '''
diff --git a/releasenotes/notes/bug-1892361-pci-deivce-type-update-c407a66fd37f6405.yaml b/releasenotes/notes/bug-1892361-pci-deivce-type-update-c407a66fd37f6405.yaml
new file mode 100644
index 000000000000..ba35c25b02de
--- /dev/null
+++ b/releasenotes/notes/bug-1892361-pci-deivce-type-update-c407a66fd37f6405.yaml
@@ -0,0 +1,12 @@
+---
+fixes:
+ - |
+ Fixes `bug 1892361`_ in which the pci stat pools are not updated when an
+ existing device is enabled with SRIOV capability. Restart of nova-compute
+ service updates the pci device type from type-PCI to type-PF but the pools
+ still maintain the device type as type-PCI. And so the PF is considered for
+ allocation to instance that requests vnic_type=direct. With this fix, the
+ pci device type updates are detected and the pci stat pools are updated
+ properly.
+
+ .. _bug 1892361: https://bugs.launchpad.net/nova/+bug/1892361