Dan Kenigsberg has posted comments on this change. Change subject: net: wait for restored devices to be up ......................................................................
Patch Set 2: Code-Review-1 (4 comments) https://gerrit.ovirt.org/#/c/43209/2/vdsm/vdsm-restore-net-config File vdsm/vdsm-restore-net-config: Line 42: from network import configurators Line 43: Line 44: import pkgutil Line 45: Line 46: _ALL_DEVICES_UP_TIMEOUT = 30 why 30 seconds? does it take so long for bonds to become UP? Line 47: Line 48: _NETS_RESTORED_MARK = os.path.join(P_VDSM_RUN, 'nets_restored') Line 49: _VIRTUAL_FUNCTIONS_PATH = os.path.join(CONF_PERSIST_DIR, 'virtual_functions') Line 50: Line 254: Line 255: Line 256: def _wait_for_for_all_devices_up(links): Line 257: down_links = set([l for l in ipwrapper.getLinks() if Line 258: l.name in links and not l.oper_up]) should we blindly wait for all links? I worry about some unrelated dummy device, causing us to be stuck forever. In task_wait_for_network we take care to wait only to devices with vdsm-created header in their ifcfg. Line 259: Line 260: if not down_links: Line 261: logging.debug("all devices are up.") Line 262: return Line 263: Line 264: logging.debug("waiting for %s to be up.", down_links) Line 265: monitor = Monitor(('link',), timeout=_ALL_DEVICES_UP_TIMEOUT, Line 266: silent_timeout=True) Line 267: monitor.start() you must start the monitor before collecting down_links. Otherwise, a race can cause one of them to receive its UP event before monitor can notice it. Line 268: for event in monitor: Line 269: if event['state'] == netinfo.OPERSTATE_UP: Line 270: down_links.discard(event['name']) Line 271: if not down_links: Line 268: for event in monitor: Line 269: if event['state'] == netinfo.OPERSTATE_UP: Line 270: down_links.discard(event['name']) Line 271: if not down_links: Line 272: break please log if we exit with a timeout. Line 273: Line 274: Line 275: def _get_all_configurators(): Line 276: """Returns the class objects of all the configurators in the netconf pkg""" -- To view, visit https://gerrit.ovirt.org/43209 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3cd3de577e5d0bcf5e87c4894e94e03c209ce76a Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ido Barkan <ibar...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Ido Barkan <ibar...@redhat.com> Gerrit-Reviewer: Jenkins CI 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
