openstack
/
nova
Code Issues Proposed changes

PCI: do not force remove allocated devices 

In the ocata release the pci_passthrough_whitelist
was moved from the [DEFAULT] section of the nova.conf
to the [pci] section and renamed to passthrough_whitelist.

On upgrading if the operator chooses to migrate the config
value to the new section it is not uncommon
to forget to rename the config value.
Similarly if an operator is updateing the whitelist and
mistypes the value it can also lead to the whitelist
being ignored.

As a result of either error the nova compute agent
would delete all database entries for a host regardless of
if the pci device was in use by an instance. If this occurs
the only recorse for an operator is to delete and recreate
the guest on that host after correcting the error or manually
restore the database to backup or otherwise consistent state.

This change alters the _set_hvdevs function to not force
remove allocated or claimed devices if they are no longer
present in the pci whitelist.

Conflicts:
  nova/pci/manager.py

Closes-Bug: #1633120
Change-Id: I6e871311a0fa10beaf601ca6912b4a33ba4094e0
(cherry picked from commit 26c41eccad)
This commit is contained in:
Sean Mooney 2018-12-19 19:40:05 +00:00
parent 9b76fc7a0a
commit 5c5a6b93a0
2 changed files with 78 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
nova/pci/manager.py
View File

@ -165,23 +165,48 @@ class PciDevTracker(object):
for existed in self.pci_devs:
if existed.address in exist_addrs - new_addrs:
# Remove previously tracked PCI devices that are either
# no longer reported by the hypervisor or have been removed
# from the pci whitelist.
try:
existed.remove()
except exception.PciDeviceInvalidStatus as e:
LOG.warning(_LW("Trying to remove device with %(status)s "
LOG.warning(_LW("Unable to remove device with %(status)s "
"ownership %(instance_uuid)s because of "
"%(pci_exception)s"),
"%(pci_exception)s. "
"Check your [pci]passthrough_whitelist "
"configuration to make sure this "
"allocated device is whitelisted. If you "
"have removed the device from the "
"whitelist intentionally or the device is "
"no longer available on the host you will "
"need to delete the server or migrate it "
"to another host to silence this warning."
),
{'status': existed.status,
'instance_uuid': existed.instance_uuid,
'pci_exception': e.format_message()})
# Note(yjiang5): remove the device by force so that
# db entry is cleaned in next sync.
existed.status = fields.PciDeviceStatus.REMOVED
# NOTE(sean-k-mooney): the device may not be tracked for
# two reasons: first the device could have been removed
# from the host or second the whitelist could have been
# updated. While force removing may seam reasonable, if
# the device is allocated to a vm, force removing the
# device entry from the resource tracker can prevent the vm
# from rebooting. If the PCI device was removed due to an
# update to the PCI whitelist which was later reverted,
# removing the entry from the database and adding it back
# later may lead to the scheduler incorrectly selecting
# this host and the ResourceTracker assigning the PCI
# device to a second vm. To prevent this bug we skip
# deleting the device from the db in this iteration and
# will try again on the next sync.
continue
else:
# Note(yjiang5): no need to update stats if an assigned
# device is hot removed.
self.stats.remove_device(existed)
else:
# Update tracked devices.
new_value = next((dev for dev in devices if
dev['address'] == existed.address))
new_value['compute_node_id'] = self.node_id
@ -204,6 +229,7 @@ class PciDevTracker(object):
else:
existed.update_device(new_value)
# Track newly discovered devices.
for dev in [dev for dev in devices if
dev['address'] in new_addrs - exist_addrs]:
dev['compute_node_id'] = self.node_id

47
nova/tests/unit/pci/test_manager.py
View File

@ -306,6 +306,53 @@ class PciDevTrackerTestCase(test.NoDBTestCase):
self.assertEqual(vfs, pf.child_devices)
self.assertEqual(vfs[0].parent_device, pf)
def test_set_hvdev_remove_tree_maintained_with_allocations(self):
# Make sure the device tree is properly maintained when there are
# devices removed from the system that are allocated to vms.
all_devs = fake_db_devs_tree[:]
self._create_tracker(all_devs)
# we start with 3 devices
self.assertEqual(
3,
len([dev for dev in self.tracker.pci_devs
if dev.status != fields.PciDeviceStatus.REMOVED]))
# we then allocate one device
pci_requests_obj = self._create_pci_requests_object(
[{'count': 1, 'spec': [{'vendor_id': 'v2'}]}])
# NOTE(sean-k-mooney): context, pci request, numa topology
claimed_dev = self.tracker.claim_instance(
mock.sentinel.context, pci_requests_obj, None)[0]
self.tracker._set_hvdevs(all_devs)
# and assert that no devices were removed
self.assertEqual(
0,
len([dev for dev in self.tracker.pci_devs
if dev.status == fields.PciDeviceStatus.REMOVED]))
# we then try to remove the allocated device from the set reported
# by the driver.
fake_pci_devs = [dev for dev in all_devs
if dev['address'] != claimed_dev.address]
with mock.patch("nova.pci.manager.LOG.warning") as log:
self.tracker._set_hvdevs(fake_pci_devs)
log.assert_called_once()
args = log.call_args_list[0][0] # args of first call
self.assertIn('Unable to remove device with', args[0])
# and assert no devices are removed from the tracker
self.assertEqual(
0,
len([dev for dev in self.tracker.pci_devs
if dev.status == fields.PciDeviceStatus.REMOVED]))
# free the device that was allocated and update tracker again
self.tracker._free_device(claimed_dev)
self.tracker._set_hvdevs(fake_pci_devs)
# and assert that one device is removed from the tracker
self.assertEqual(
1,
len([dev for dev in self.tracker.pci_devs
if dev.status == fields.PciDeviceStatus.REMOVED]))
def test_set_hvdev_changed_stal(self):
pci_requests_obj = self._create_pci_requests_object(
[{'count': 1, 'spec': [{'vendor_id': 'v1'}]}])