From 9d8cdc5bb33f9e8ae4dfd658a0c9b216e7557431 Mon Sep 17 00:00:00 2001 From: Douglas Henrique Koerich Date: Fri, 14 May 2021 14:45:29 -0400 Subject: [PATCH] Replace applying flag by dict for FEC device check This change replaces the latest solution to check FEC device before an unlock action, that relied on an '/APPLYING' flag. In certain asynchro- nous scenarios, that flag could be cleared before than expected if an inventory report not related to the FEC device configuration came late (that might happen when configuring a long queue of SRIOV port changes) or by periodic sysinv report. The solution still uses the 'extra_info' field of PCI devices, this time "stringifying" a dictionary entry for 'expected_numvfs' that will keep (without clearing) at that field the programmed number of VFs at FEC device. It is then compared with the actual sriov_numvfs of device from the inventory report, in a similar way of what is currently done for comparing SRIOV interfaces (from database) to ports (from device). Closes-bug: 1927089 Signed-off-by: Douglas Henrique Koerich Change-Id: I380bd66a8229a72ef1981cbefa3a0543c28d7f30 --- .../sysinv/sysinv/api/controllers/v1/host.py | 47 +++++++++++-------- .../sysinv/api/controllers/v1/pci_device.py | 20 ++++---- sysinv/sysinv/sysinv/sysinv/common/device.py | 3 -- .../sysinv/sysinv/sysinv/conductor/manager.py | 12 ----- 4 files changed, 40 insertions(+), 42 deletions(-) diff --git a/sysinv/sysinv/sysinv/sysinv/api/controllers/v1/host.py b/sysinv/sysinv/sysinv/sysinv/api/controllers/v1/host.py index 62f1547b59..27983c2cae 100644 --- a/sysinv/sysinv/sysinv/sysinv/api/controllers/v1/host.py +++ b/sysinv/sysinv/sysinv/sysinv/api/controllers/v1/host.py @@ -3397,29 +3397,38 @@ class HostController(rest.RestController): dev.pdevice_id not in device.SRIOV_ENABLED_FEC_DEVICE_IDS): return - if (dev.extra_info and - dev.extra_info.endswith(device.DEVICE_APPLY_PENDING)): - msg = (_("Pending configuration of FEC device. " - "Please wait a few minutes for inventory update and " - "retry host-unlock.")) - LOG.info(msg) - pecan.request.rpcapi.update_sriov_config( - pecan.request.context, - host['uuid']) - raise wsme.exc.ClientSideError(msg) + try: + sriov_numvfs = int(dev.sriov_numvfs) + except TypeError: + sriov_numvfs = 0 - sriov_numvfs = dev.sriov_numvfs - if not sriov_numvfs: - return - if (dev.sriov_vfs_pci_address and - sriov_numvfs == len(dev.sriov_vfs_pci_address.split(','))): + if dev.extra_info: + extra_info = ast.literal_eval(dev.extra_info) + expected_numvfs = int(extra_info['expected_numvfs']) + if sriov_numvfs != expected_numvfs: + msg = (_("Expecting sriov_numvfs=%d for FEC device pciaddr=%s. " + "Please wait a few minutes for inventory update and " + "retry host-unlock." % (expected_numvfs, dev.pciaddr))) + LOG.info(msg) + pecan.request.rpcapi.update_sriov_config( + pecan.request.context, + host['uuid']) + raise wsme.exc.ClientSideError(msg) + + if not dev.sriov_vfs_pci_address or len(dev.sriov_vfs_pci_address) == 0: + sriov_vfs_pci_address = [] + else: + sriov_vfs_pci_address = dev.sriov_vfs_pci_address.split(',') + + if sriov_numvfs == len(sriov_vfs_pci_address): + if sriov_numvfs > 0: LOG.info("check sriov_numvfs=%s sriov_vfs_pci_address=%s" % (sriov_numvfs, dev.sriov_vfs_pci_address)) else: - msg = (_("Expecting number of FEC device sriov_numvfs=%s. " - "Please wait a few minutes for inventory update and " - "retry host-unlock." % - sriov_numvfs)) + msg = (_("Expecting sriov_vfs_pci_address length=%d for FEC " + "device pciaddr=%s. Please wait a few minutes for " + "inventory update and retry host-unlock." % + (sriov_numvfs, dev.pciaddr))) LOG.info(msg) pecan.request.rpcapi.update_sriov_config( pecan.request.context, diff --git a/sysinv/sysinv/sysinv/sysinv/api/controllers/v1/pci_device.py b/sysinv/sysinv/sysinv/sysinv/api/controllers/v1/pci_device.py index b49f5deaf4..c62d31c7e8 100755 --- a/sysinv/sysinv/sysinv/sysinv/api/controllers/v1/pci_device.py +++ b/sysinv/sysinv/sysinv/sysinv/api/controllers/v1/pci_device.py @@ -9,6 +9,7 @@ from pecan import rest import wsme from wsme import types as wtypes import wsmeext.pecan as wsme_pecan +from ast import literal_eval from oslo_log import log from sysinv._i18n import _ @@ -327,13 +328,16 @@ class PCIDeviceController(rest.RestController): rpc_device[field] = None else: rpc_device[field] = getattr(device, field) - - if sriov_update: - # Set indication of pending configuration (runtime manifest apply) - # for this device - if not rpc_device['extra_info']: - rpc_device['extra_info'] = '' - rpc_device['extra_info'] += dconstants.DEVICE_APPLY_PENDING + if field == 'sriov_numvfs': + # Save desired number of VFs in extra_info since + # sriov_numvfs may get overwritten by concurrent inventory report + expected_numvfs = {'expected_numvfs': rpc_device[field]} + if not rpc_device['extra_info']: + rpc_device['extra_info'] = str(expected_numvfs) + else: + extra_info = literal_eval(rpc_device['extra_info']) + extra_info.update(expected_numvfs) + rpc_device['extra_info'] = str(extra_info) rpc_device.save() @@ -355,7 +359,7 @@ def _check_host(host): def _check_field(field): - if field not in ["enabled", "name", "driver", "sriov_numvfs", "sriov_vf_driver"]: + if field not in ["enabled", "name", "driver", "sriov_numvfs", "sriov_vf_driver", "extra_info"]: raise wsme.exc.ClientSideError(_('Modifying %s attribute restricted') % field) diff --git a/sysinv/sysinv/sysinv/sysinv/common/device.py b/sysinv/sysinv/sysinv/sysinv/common/device.py index 9fd92b4daa..74cd1684e4 100644 --- a/sysinv/sysinv/sysinv/sysinv/common/device.py +++ b/sysinv/sysinv/sysinv/sysinv/common/device.py @@ -89,6 +89,3 @@ DEVICE_IMAGE_UPDATE_NULL = '' # Device Image Action APPLY_ACTION = 'apply' REMOVE_ACTION = 'remove' - -# Device Configuration Status -DEVICE_APPLY_PENDING = '/APPLYING' diff --git a/sysinv/sysinv/sysinv/sysinv/conductor/manager.py b/sysinv/sysinv/sysinv/sysinv/conductor/manager.py index 3d274b0d73..343c3b49b2 100644 --- a/sysinv/sysinv/sysinv/sysinv/conductor/manager.py +++ b/sysinv/sysinv/sysinv/sysinv/conductor/manager.py @@ -2804,18 +2804,6 @@ class ConductorManager(service.PeriodicService): 'extra_info': dev.get('extra_info', None)} LOG.info("attr: %s" % attr) - if attr['extra_info']: - extra_info = attr['extra_info'] - - # TODO: Change 'extra_info' to be a string representation - # of a dictionary, embedding the desired (requested) #VFs - # along with the APPLYING flag - - # Get rid of indication of pending configuration, if any - if extra_info.endswith(dconstants.DEVICE_APPLY_PENDING): - attr['extra_info'] = extra_info[:extra_info.find( - dconstants.DEVICE_APPLY_PENDING)] - if (host['administrative'] == constants.ADMIN_LOCKED and pci_dev['pdevice_id'] in dconstants.SRIOV_ENABLED_FEC_DEVICE_IDS):