openstack
/
nova
Code Issues Proposed changes

pci: Use fields.Enum type for PCI device status 

Adds a new field type specialization for PCI device status, allowing
us to remove a bunch of hard-coded strings.

Change-Id: Ideca8547a693f0560d2a94d6616a43f0ce327ec8
This commit is contained in:
Jay Pipes 2015-06-14 14:10:06 -04:00 committed by Dan Smith
parent 9a3a8209de
commit cac9d52d07
11 changed files with 106 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
nova/objects/fields.py
View File

@ -380,6 +380,21 @@ class VersionPredicate(fields.String):
return value
class PciDeviceStatus(Enum):
AVAILABLE = "available"
CLAIMED = "claimed"
ALLOCATED = "allocated"
REMOVED = "removed" # The device has been hot-removed and not yet deleted
DELETED = "deleted" # The device is marked not available/deleted.
ALL = (AVAILABLE, CLAIMED, ALLOCATED, REMOVED, DELETED)
def __init__(self):
super(PciDeviceStatus, self).__init__(
valid_values=PciDeviceStatus.ALL)
# NOTE(danms): Remove this on next release of oslo.versionedobjects
class FlexibleBoolean(fields.Boolean):
@staticmethod
@ -652,6 +667,10 @@ class VersionPredicateField(AutoTypedField):
AUTO_TYPE = VersionPredicate()
class PciDeviceStatusField(BaseEnumField):
AUTO_TYPE = PciDeviceStatus()
# FIXME(danms): Remove this after oslo.versionedobjects gets it
# This is a flexible interpretation of boolean
# values using common user friendly semantics for

10
nova/objects/pci_device.py
View File

@ -97,7 +97,7 @@ class PciDevice(base.NovaPersistentObject, base.NovaObject):
'vendor_id': fields.StringField(),
'product_id': fields.StringField(),
'dev_type': fields.StringField(),
'status': fields.StringField(),
'status': fields.PciDeviceStatusField(),
'dev_id': fields.StringField(nullable=True),
'label': fields.StringField(nullable=True),
'instance_uuid': fields.StringField(nullable=True),
@ -179,16 +179,16 @@ class PciDevice(base.NovaPersistentObject, base.NovaObject):
"""
pci_device = cls()
pci_device.update_device(dev_dict)
pci_device.status = 'available'
pci_device.status = fields.PciDeviceStatus.AVAILABLE
return pci_device
@base.remotable
def save(self):
if self.status == 'removed':
self.status = 'deleted'
if self.status == fields.PciDeviceStatus.REMOVED:
self.status = fields.PciDeviceStatus.DELETED
db.pci_device_destroy(self._context, self.compute_node_id,
self.address)
elif self.status != 'deleted':
elif self.status != fields.PciDeviceStatus.DELETED:
updates = self.obj_get_changes()
if 'extra_info' in updates:
updates['extra_info'] = jsonutils.dumps(updates['extra_info'])

28
nova/pci/device.py
View File

@ -17,6 +17,7 @@ import copy
import functools
from nova import exception
from nova.objects import fields
def check_device_status(dev_status=None):
@ -41,21 +42,25 @@ def check_device_status(dev_status=None):
return outer
@check_device_status(dev_status=['available'])
@check_device_status(dev_status=[fields.PciDeviceStatus.AVAILABLE])
def claim(devobj, instance):
devobj.status = 'claimed'
devobj.status = fields.PciDeviceStatus.CLAIMED
devobj.instance_uuid = instance['uuid']
@check_device_status(dev_status=['available', 'claimed'])
@check_device_status(dev_status=[
fields.PciDeviceStatus.AVAILABLE,
fields.PciDeviceStatus.CLAIMED
])
def allocate(devobj, instance):
if devobj.status == 'claimed' and devobj.instance_uuid != instance['uuid']:
if (devobj.status == fields.PciDeviceStatus.CLAIMED and
devobj.instance_uuid != instance['uuid']):
raise exception.PciDeviceInvalidOwner(
compute_node_id=devobj.compute_node_id,
address=devobj.address, owner=devobj.instance_uuid,
hopeowner=instance['uuid'])
devobj.status = 'allocated'
devobj.status = fields.PciDeviceStatus.ALLOCATED
devobj.instance_uuid = instance['uuid']
# Notes(yjiang5): remove this check when instance object for
@ -68,14 +73,17 @@ def allocate(devobj, instance):
instance.pci_devices.objects.append(copy.copy(devobj))
@check_device_status(dev_status=['available'])
@check_device_status(dev_status=[fields.PciDeviceStatus.AVAILABLE])
def remove(devobj):
devobj.status = 'removed'
devobj.status = fields.PciDeviceStatus.REMOVED
devobj.instance_uuid = None
devobj.request_id = None
@check_device_status(dev_status=['claimed', 'allocated'])
@check_device_status(dev_status=[
fields.PciDeviceStatus.CLAIMED,
fields.PciDeviceStatus.ALLOCATED
])
def free(devobj, instance=None):
if instance and devobj.instance_uuid != instance['uuid']:
raise exception.PciDeviceInvalidOwner(
@ -83,10 +91,10 @@ def free(devobj, instance=None):
address=devobj.address, owner=devobj.instance_uuid,
hopeowner=instance['uuid'])
old_status = devobj.status
devobj.status = 'available'
devobj.status = fields.PciDeviceStatus.AVAILABLE
devobj.instance_uuid = None
devobj.request_id = None
if old_status == 'allocated' and instance:
if old_status == fields.PciDeviceStatus.ALLOCATED and instance:
# Notes(yjiang5): remove this check when instance object for
# compute manager is finished
existed = next((dev for dev in instance['pci_devices']

21
nova/pci/manager.py
View File

@ -21,6 +21,7 @@ from oslo_log import log as logging
from nova import exception
from nova.i18n import _LW
from nova import objects
from nova.objects import fields
from nova.pci import device
from nova.pci import stats
from nova.virt import hardware
@ -64,11 +65,11 @@ class PciDevTracker(object):
self.claims = collections.defaultdict(list)
for dev in self.pci_devs:
uuid = dev.instance_uuid
if dev.status == 'claimed':
if dev.status == fields.PciDeviceStatus.CLAIMED:
self.claims[uuid].append(dev)
elif dev.status == 'allocated':
elif dev.status == fields.PciDeviceStatus.ALLOCATED:
self.allocations[uuid].append(dev)
elif dev.status == 'available':
elif dev.status == fields.PciDeviceStatus.AVAILABLE:
self.stats.add_device(dev)
@property
@ -82,7 +83,7 @@ class PciDevTracker(object):
dev.save()
self.pci_devs = [dev for dev in self.pci_devs
if dev.status != 'deleted']
if dev.status != fields.PciDeviceStatus.DELETED]
@property
def pci_stats(self):
@ -117,7 +118,7 @@ class PciDevTracker(object):
'pci_exception': e.format_message()})
# Note(yjiang5): remove the device by force so that
# db entry is cleaned in next sync.
existed.status = 'removed'
existed.status = fields.PciDeviceStatus.REMOVED
else:
# Note(yjiang5): no need to update stats if an assigned
# device is hot removed.
@ -126,7 +127,8 @@ class PciDevTracker(object):
new_value = next((dev for dev in devices if
dev['address'] == existed.address))
new_value['compute_node_id'] = self.node_id
if existed.status in ('claimed', 'allocated'):
if existed.status in (fields.PciDeviceStatus.CLAIMED,
fields.PciDeviceStatus.ALLOCATED):
# Pci properties may change while assigned because of
# hotplug or config changes. Although normally this should
# not happen.
@ -210,9 +212,10 @@ class PciDevTracker(object):
# information, not the claimed one. So we can't use
# instance['pci_devices'] to check the devices to be freed.
for dev in self.pci_devs:
if (dev.status in ('claimed', 'allocated') and
dev.instance_uuid == instance['uuid']):
self._free_device(dev)
if dev.status in (fields.PciDeviceStatus.CLAIMED,
fields.PciDeviceStatus.ALLOCATED):
if dev.instance_uuid == instance['uuid']:
self._free_device(dev)
def free_instance(self, context, instance):
if self.allocations.pop(instance['uuid'], None):

19
nova/tests/unit/db/test_db_api.py
View File

@ -59,6 +59,7 @@ from nova.db.sqlalchemy import types as col_types
from nova.db.sqlalchemy import utils as db_utils
from nova import exception
from nova import objects
from nova.objects import fields
from nova import quota
from nova import test
from nova.tests.unit import matchers
@ -6111,7 +6112,7 @@ class NetworkTestCase(test.TestCase, ModelsObjectComparatorMixin):
self.assertEqual('192.0.2.1', data[0]['address'])
self.assertEqual('192.0.2.1', data[0]['vif_address'])
self.assertEqual(instance.uuid, data[0]['instance_uuid'])
self.assertTrue(data[0]['allocated'])
self.assertTrue(data[0][fields.PciDeviceStatus.ALLOCATED])
def test_network_create_safe(self):
values = {'host': 'localhost', 'project_id': 'project1'}
@ -8473,7 +8474,7 @@ class PciDeviceDBApiTestCase(test.TestCase, ModelsObjectComparatorMixin):
'dev_id': 'pci_0000:0f:08.7',
'extra_info': None,
'label': 'label_8086_1520',
'status': 'available',
'status': fields.PciDeviceStatus.AVAILABLE,
'instance_uuid': '00000000-0000-0000-0000-000000000010',
'request_id': None,
}, {'id': 3356,
@ -8486,7 +8487,7 @@ class PciDeviceDBApiTestCase(test.TestCase, ModelsObjectComparatorMixin):
'dev_id': 'pci_0000:0f:08.7',
'extra_info': None,
'label': 'label_8086_1520',
'status': 'available',
'status': fields.PciDeviceStatus.AVAILABLE,
'instance_uuid': '00000000-0000-0000-0000-000000000010',
'request_id': None,
}
@ -8554,8 +8555,8 @@ class PciDeviceDBApiTestCase(test.TestCase, ModelsObjectComparatorMixin):
def test_pci_device_get_by_instance_uuid(self):
v1, v2 = self._create_fake_pci_devs()
v1['status'] = 'allocated'
v2['status'] = 'allocated'
v1['status'] = fields.PciDeviceStatus.ALLOCATED
v2['status'] = fields.PciDeviceStatus.ALLOCATED
db.pci_device_update(self.admin_context, v1['compute_node_id'],
v1['address'], v1)
db.pci_device_update(self.admin_context, v2['compute_node_id'],
@ -8567,8 +8568,8 @@ class PciDeviceDBApiTestCase(test.TestCase, ModelsObjectComparatorMixin):
def test_pci_device_get_by_instance_uuid_check_status(self):
v1, v2 = self._create_fake_pci_devs()
v1['status'] = 'allocated'
v2['status'] = 'claimed'
v1['status'] = fields.PciDeviceStatus.ALLOCATED
v2['status'] = fields.PciDeviceStatus.CLAIMED
db.pci_device_update(self.admin_context, v1['compute_node_id'],
v1['address'], v1)
db.pci_device_update(self.admin_context, v2['compute_node_id'],
@ -8580,14 +8581,14 @@ class PciDeviceDBApiTestCase(test.TestCase, ModelsObjectComparatorMixin):
def test_pci_device_update(self):
v1, v2 = self._create_fake_pci_devs()
v1['status'] = 'allocated'
v1['status'] = fields.PciDeviceStatus.ALLOCATED
db.pci_device_update(self.admin_context, v1['compute_node_id'],
v1['address'], v1)
result = db.pci_device_get_by_addr(
self.admin_context, 1, '0000:0f:08.7')
self._assertEqualObjects(v1, result, self.ignored_keys)
v1['status'] = 'claimed'
v1['status'] = fields.PciDeviceStatus.CLAIMED
db.pci_device_update(self.admin_context, v1['compute_node_id'],
v1['address'], v1)
result = db.pci_device_get_by_addr(

5
nova/tests/unit/objects/test_instance.py
View File

@ -29,6 +29,7 @@ from nova.network import model as network_model
from nova import notifications
from nova import objects
from nova.objects import base
from nova.objects import fields
from nova.objects import instance
from nova.objects import instance_info_cache
from nova.objects import pci_device
@ -741,7 +742,7 @@ class _TestInstanceObject(object):
'numa_node': 0,
'product_id': 'p1',
'dev_type': 't',
'status': 'allocated',
'status': fields.PciDeviceStatus.ALLOCATED,
'dev_id': 'i',
'label': 'l',
'instance_uuid': fake_uuid,
@ -759,7 +760,7 @@ class _TestInstanceObject(object):
'numa_node': 1,
'product_id': 'p',
'dev_type': 't',
'status': 'allocated',
'status': fields.PciDeviceStatus.ALLOCATED,
'dev_id': 'i',
'label': 'l',
'instance_uuid': fake_uuid,

2
nova/tests/unit/objects/test_objects.py
View File

@ -1145,7 +1145,7 @@ object_data = {
'NetworkList': '1.2-69eca910d8fa035dfecd8ba10877ee59',
'NetworkRequest': '1.1-7a3e4ca2ce1e7b62d8400488f2f2b756',
'NetworkRequestList': '1.1-15ecf022a68ddbb8c2a6739cfc9f8f5e',
'PciDevice': '1.3-4d43db45e3978fca4280f696633c7c20',
'PciDevice': '1.3-9781610624d20acc48da992567b7f397',
'PciDeviceList': '1.2-3757458c45591cbc92c72ee99e757c98',
'PciDevicePool': '1.1-3f5ddc3ff7bfa14da7f6c7e9904cc000',
'PciDevicePoolList': '1.1-15ecf022a68ddbb8c2a6739cfc9f8f5e',

31
nova/tests/unit/objects/test_pci_device.py
View File

@ -19,6 +19,7 @@ from oslo_utils import timeutils
from nova import context
from nova import db
from nova.objects import fields
from nova.objects import instance
from nova.objects import pci_device
from nova.tests.unit.objects import test_objects
@ -29,7 +30,7 @@ dev_dict = {
'product_id': 'p',
'vendor_id': 'v',
'numa_node': 0,
'status': 'available'}
'status': fields.PciDeviceStatus.AVAILABLE}
fake_db_dev = {
@ -44,7 +45,7 @@ fake_db_dev = {
'product_id': 'p',
'numa_node': 0,
'dev_type': 't',
'status': 'available',
'status': fields.PciDeviceStatus.AVAILABLE,
'dev_id': 'i',
'label': 'l',
'instance_uuid': None,
@ -65,7 +66,7 @@ fake_db_dev_1 = {
'product_id': 'p1',
'numa_node': 1,
'dev_type': 't',
'status': 'available',
'status': fields.PciDeviceStatus.AVAILABLE,
'dev_id': 'i',
'label': 'l',
'instance_uuid': None,
@ -148,23 +149,24 @@ class _TestPciDeviceObject(object):
def test_save(self):
ctxt = context.get_admin_context()
self._create_fake_pci_device(ctxt=ctxt)
return_dev = dict(fake_db_dev, status='available',
return_dev = dict(fake_db_dev, status=fields.PciDeviceStatus.AVAILABLE,
instance_uuid='fake-uuid-3')
self.pci_device.status = 'allocated'
self.pci_device.status = fields.PciDeviceStatus.ALLOCATED
self.pci_device.instance_uuid = 'fake-uuid-2'
expected_updates = dict(status='allocated',
expected_updates = dict(status=fields.PciDeviceStatus.ALLOCATED,
instance_uuid='fake-uuid-2')
self.mox.StubOutWithMock(db, 'pci_device_update')
db.pci_device_update(ctxt, 1, 'a',
expected_updates).AndReturn(return_dev)
self.mox.ReplayAll()
self.pci_device.save()
self.assertEqual(self.pci_device.status, 'available')
self.assertEqual(self.pci_device.status,
fields.PciDeviceStatus.AVAILABLE)
self.assertEqual(self.pci_device.instance_uuid,
'fake-uuid-3')
def test_save_no_extra_info(self):
return_dev = dict(fake_db_dev, status='available',
return_dev = dict(fake_db_dev, status=fields.PciDeviceStatus.AVAILABLE,
instance_uuid='fake-uuid-3')
def _fake_update(ctxt, node_id, addr, updates):
@ -181,12 +183,13 @@ class _TestPciDeviceObject(object):
def test_save_removed(self):
ctxt = context.get_admin_context()
self._create_fake_pci_device(ctxt=ctxt)
self.pci_device.status = 'removed'
self.pci_device.status = fields.PciDeviceStatus.REMOVED
self.mox.StubOutWithMock(db, 'pci_device_destroy')
db.pci_device_destroy(ctxt, 1, 'a')
self.mox.ReplayAll()
self.pci_device.save()
self.assertEqual(self.pci_device.status, 'deleted')
self.assertEqual(self.pci_device.status,
fields.PciDeviceStatus.DELETED)
def test_save_deleted(self):
def _fake_destroy(ctxt, node_id, addr):
@ -197,7 +200,7 @@ class _TestPciDeviceObject(object):
self.stubs.Set(db, 'pci_device_destroy', _fake_destroy)
self.stubs.Set(db, 'pci_device_update', _fake_update)
self._create_fake_pci_device()
self.pci_device.status = 'deleted'
self.pci_device.status = fields.PciDeviceStatus.DELETED
self.called = False
self.pci_device.save()
self.assertEqual(self.called, False)
@ -270,9 +273,11 @@ class _TestPciDeviceListObject(object):
def test_get_by_instance_uuid(self):
ctxt = context.get_admin_context()
fake_db_1 = dict(fake_db_dev, address='a1',
status='allocated', instance_uuid='1')
status=fields.PciDeviceStatus.ALLOCATED,
instance_uuid='1')
fake_db_2 = dict(fake_db_dev, address='a2',
status='allocated', instance_uuid='1')
status=fields.PciDeviceStatus.ALLOCATED,
instance_uuid='1')
self.mox.StubOutWithMock(db, 'pci_device_get_all_by_instance_uuid')
db.pci_device_get_all_by_instance_uuid(ctxt, '1').AndReturn(
[fake_db_1, fake_db_2])

19
nova/tests/unit/pci/test_device.py
View File

@ -16,6 +16,7 @@
from nova import context
from nova import exception
from nova import objects
from nova.objects import fields
from nova.pci import device
from nova import test
@ -32,7 +33,7 @@ dev_dict = {
'product_id': 'p',
'numa_node': 1,
'dev_type': 't',
'status': 'available',
'status': fields.PciDeviceStatus.AVAILABLE,
'dev_id': 'i',
'label': 'l',
'instance_uuid': None,
@ -55,26 +56,28 @@ class PciDeviceTestCase(test.NoDBTestCase):
def test_claim_device(self):
device.claim(self.devobj, self.inst)
self.assertEqual(self.devobj.status, 'claimed')
self.assertEqual(self.devobj.status,
fields.PciDeviceStatus.CLAIMED)
self.assertEqual(self.devobj.instance_uuid,
self.inst.uuid)
self.assertEqual(len(self.inst.pci_devices), 0)
def test_claim_device_fail(self):
self.devobj.status = 'allocated'
self.devobj.status = fields.PciDeviceStatus.ALLOCATED
self.assertRaises(exception.PciDeviceInvalidStatus,
device.claim, self.devobj, self.inst)
def test_allocate_device(self):
device.claim(self.devobj, self.inst)
device.allocate(self.devobj, self.inst)
self.assertEqual(self.devobj.status, 'allocated')
self.assertEqual(self.devobj.status,
fields.PciDeviceStatus.ALLOCATED)
self.assertEqual(self.devobj.instance_uuid, 'fake-inst-uuid')
self.assertEqual(len(self.inst.pci_devices), 1)
self.assertEqual(self.inst.pci_devices[0].vendor_id,
'v')
self.assertEqual(self.inst.pci_devices[0].status,
'allocated')
fields.PciDeviceStatus.ALLOCATED)
def test_allocacte_device_fail_status(self):
self.devobj.status = 'removed'
@ -94,7 +97,8 @@ class PciDeviceTestCase(test.NoDBTestCase):
def test_free_claimed_device(self):
device.claim(self.devobj, self.inst)
device.free(self.devobj, self.inst)
self.assertEqual(self.devobj.status, 'available')
self.assertEqual(self.devobj.status,
fields.PciDeviceStatus.AVAILABLE)
self.assertIsNone(self.devobj.instance_uuid)
def test_free_allocated_device(self):
@ -103,7 +107,8 @@ class PciDeviceTestCase(test.NoDBTestCase):
self.assertEqual(len(self.inst.pci_devices), 1)
device.free(self.devobj, self.inst)
self.assertEqual(len(self.inst.pci_devices), 0)
self.assertEqual(self.devobj.status, 'available')
self.assertEqual(self.devobj.status,
fields.PciDeviceStatus.AVAILABLE)
self.assertIsNone(self.devobj.instance_uuid)
def test_free_device_fail(self):

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

@ -22,6 +22,7 @@ from nova.compute import vm_states
from nova import context
from nova import db
from nova import objects
from nova.objects import fields
from nova.pci import device
from nova.pci import manager
from nova import test
@ -35,7 +36,7 @@ fake_pci = {
'product_id': 'p',
'vendor_id': 'v',
'request_id': None,
'status': 'available',
'status': fields.PciDeviceStatus.AVAILABLE,
'numa_node': 0}
fake_pci_1 = dict(fake_pci, address='0000:00:00.2',
product_id='p1', vendor_id='v1')
@ -54,7 +55,7 @@ fake_db_dev = {
'product_id': 'p',
'numa_node': 1,
'dev_type': 't',
'status': 'available',
'status': fields.PciDeviceStatus.AVAILABLE,
'dev_id': 'i',
'label': 'l',
'instance_uuid': None,

9
nova/tests/unit/virt/libvirt/test_driver.py
View File

@ -62,6 +62,7 @@ from nova import db
from nova import exception
from nova.network import model as network_model
from nova import objects
from nova.objects import fields
from nova.pci import manager as pci_manager
from nova import test
from nova.tests.unit import fake_block_device
@ -1419,7 +1420,7 @@ class LibvirtConnTestCase(test.NoDBTestCase):
pci_device_info = dict(test_pci_device.fake_db_dev)
pci_device_info.update(compute_node_id=1,
label='fake',
status='available',
status=fields.PciDeviceStatus.AVAILABLE,
address='0000:00:00.1',
instance_uuid=None,
request_id=None,
@ -1464,7 +1465,7 @@ class LibvirtConnTestCase(test.NoDBTestCase):
pci_device_info = dict(test_pci_device.fake_db_dev)
pci_device_info.update(compute_node_id=1,
label='fake',
status='available',
status=fields.PciDeviceStatus.AVAILABLE,
address='0000:00:00.1',
instance_uuid=None,
request_id=None,
@ -3734,7 +3735,7 @@ class LibvirtConnTestCase(test.NoDBTestCase):
pci_device_info = dict(test_pci_device.fake_db_dev)
pci_device_info.update(compute_node_id=1,
label='fake',
status='allocated',
status=fields.PciDeviceStatus.ALLOCATED,
address='0000:00:00.1',
compute_id=compute_ref['id'],
instance_uuid=instance.uuid,
@ -3776,7 +3777,7 @@ class LibvirtConnTestCase(test.NoDBTestCase):
pci_device_info = dict(test_pci_device.fake_db_dev)
pci_device_info.update(compute_node_id=1,
label='fake',
status='allocated',
status=fields.PciDeviceStatus.ALLOCATED,
address='0000:00:00.2',
compute_id=compute_ref['id'],
instance_uuid=instance.uuid,