Ido Barkan has posted comments on this change. Change subject: network: persist sriov devices number of vfs ......................................................................
Patch Set 18: (5 comments) https://gerrit.ovirt.org/#/c/40088/18/vdsm/network/api.py File vdsm/network/api.py: Line 557: stored. A call to setSafeNetworkConfig() will persist it across reboots. Line 558: Garbage is collected during network restoration. Line 559: """ Line 560: logger = logging.getLogger("change_numvfs") Line 561: logger.info("changing number of vfs on device %s -> %s.", > there is no much use for this getLogger - it has no real context or special Done Line 562: device_name, numvfs) Line 563: _update_numvfs(device_name, numvfs) Line 564: Line 565: logger.info("changing number of vfs on device %s -> %s. succeeded.", https://gerrit.ovirt.org/#/c/40088/18/vdsm/vdsm-restore-net-config File vdsm/vdsm-restore-net-config: Line 27: Line 28: from vdsm.config import config Line 29: from vdsm import netinfo Line 30: from vdsm import utils Line 31: from vdsm.constants import P_VDSM_RUN, P_VDSM_LIB > please pull constant from vdsm.netconfpersist - "constants" is just a pile P_VDSM_RUN_NETCONF was the one I added, and now it is declared only in vdsm.netconfpersistance.py Line 32: import hostdev Line 33: Line 34: # Ifcfg persistence restoration Line 35: from network.configurators import ifcfg Line 64: missing_devices.append(file_name) Line 65: else: Line 66: with open(os.path.join( Line 67: _VIRTUAL_FUNCTIONS_PATH, file_name)) as f: Line 68: numvfs_by_device[file_name] = int(f.read().strip()) > let's use the same kind of semantics: if one PF failed, let us skip it. Done Line 69: Line 70: return numvfs_by_device, missing_devices Line 71: Line 72: Line 73: def _garbage_collect_devices_persistence(missing_devices): Line 74: for file_name in missing_devices: Line 75: path_to_remove = os.path.join(_VIRTUAL_FUNCTIONS_PATH, file_name) Line 76: logging.debug('Removing %s. Device no longer exists.', path_to_remove) Line 77: utils.rmFile(path_to_remove) > here too - let us just skip the missing VFs, log the fact (as an ERROR), bu So this function should not be called ever. Line 78: Line 79: Line 80: def _restore_sriov_numvfs(): Line 81: sriov_devices = _get_sriov_devices() Line 107: Builds a setupNetworks command from the persistent configuration to set it Line 108: as running configuration. Line 109: """ Line 110: _restore_sriov_numvfs() Line 111: > how come pep8 did not bite? removed Line 112: _remove_networks_in_running_config() Line 113: Line 114: _flush_configurators_leftovers() Line 115: -- To view, visit https://gerrit.ovirt.org/40088 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I76b898019840ffe65939ffad4a1e98829ad3c887 Gerrit-PatchSet: 18 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ido Barkan <ibar...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Ido Barkan <ibar...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik <mpoled...@redhat.com> Gerrit-Reviewer: Ondřej Svoboda <osvob...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
