Change in vdsm[master]: network: clientSeen should have it's default timeout
Dan Kenigsberg has posted comments on this change. Change subject: network: clientSeen should have it's default timeout .. Patch Set 1: Code-Review-1 (3 comments) https://gerrit.ovirt.org/#/c/40087/1//COMMIT_MSG Commit Message: Line 3: AuthorDate: 2015-04-21 12:27:50 +0300 Line 4: Commit: Ido Barkan ibar...@redhat.com Line 5: CommitDate: 2015-04-21 16:03:10 +0300 Line 6: Line 7: network: clientSeen should have it's default timeout it's - its Line 8: Line 9: Change-Id: Ibe5e28247bebd91878c2a2c687b2eeb9bfec6586 https://gerrit.ovirt.org/#/c/40087/1/vdsm/network/api.py File vdsm/network/api.py: Line 513: Line 514: Line 515: def clientSeen(timeout=CONNECTIVITY_TIMEOUT_DEFAULT): Line 516: start = time.time() Line 517: while timeout = 0: this explodes if timeout is None. Hence the frightening catching-all except of line 729 does not remove the exception, only hides its origin. Better have here if timeout is None: timeout = CONNECTIVITY_TIMEOUT_DEFAULT or move this logic into your new _get_connectivity_timeout Line 518: try: Line 519: if os.stat(constants.P_VDSM_CLIENT_LOG).st_mtime start: Line 520: return True Line 521: except OSError as e: Line 724: Line 725: def _get_connectivity_timeout(options): Line 726: try: Line 727: return int(options.get('connectivityTimeout')) Line 728: except: We must never swallow exception without a trace. Particularly in this case, when we got garbage from Engine. I prefer fully-blown traceback in such conditions. Line 729: return None Line 730: Line 731: Line 732: def _check_connectivity(connectivity_check_networks, networks, bondings, -- To view, visit https://gerrit.ovirt.org/40087 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibe5e28247bebd91878c2a2c687b2eeb9bfec6586 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ido Barkan ibar...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Ondřej Svoboda osvob...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: network: clientSeen should have it's default timeout
automat...@ovirt.org has posted comments on this change. Change subject: network: clientSeen should have it's default timeout .. Patch Set 1: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/40087 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibe5e28247bebd91878c2a2c687b2eeb9bfec6586 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ido Barkan ibar...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: network: clientSeen should have it's default timeout
oVirt Jenkins CI Server has posted comments on this change. Change subject: network: clientSeen should have it's default timeout .. Patch Set 1: Build Started (1/3) - http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/17945/ -- To view, visit https://gerrit.ovirt.org/40087 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibe5e28247bebd91878c2a2c687b2eeb9bfec6586 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ido Barkan ibar...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: network: clientSeen should have it's default timeout
Ido Barkan has uploaded a new change for review. Change subject: network: clientSeen should have it's default timeout .. network: clientSeen should have it's default timeout Change-Id: Ibe5e28247bebd91878c2a2c687b2eeb9bfec6586 Signed-off-by: Ido Barkan ibar...@redhat.com --- M vdsm/network/api.py 1 file changed, 10 insertions(+), 5 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/87/40087/1 diff --git a/vdsm/network/api.py b/vdsm/network/api.py index 7e76b44..bc2dbd2 100755 --- a/vdsm/network/api.py +++ b/vdsm/network/api.py @@ -512,7 +512,7 @@ configurator.removeQoS(net_ent) -def clientSeen(timeout): +def clientSeen(timeout=CONNECTIVITY_TIMEOUT_DEFAULT): start = time.time() while timeout = 0: try: @@ -535,8 +535,7 @@ _addNetwork(newBridge, vlan=vlan, bonding=bonding, nics=nics, configurator=configurator, **options) if utils.tobool(options.get('connectivityCheck', False)): -if not clientSeen(int(options.get('connectivityTimeout', - CONNECTIVITY_TIMEOUT_DEFAULT))): +if not clientSeen(_get_connectivity_timeout(options)): _delNetwork(newBridge, force=True) raise ConfigNetworkError(ne.ERR_LOST_CONNECTION, 'connectivity check failed') @@ -723,12 +722,18 @@ _netinfo.updateDevices() # Things like a bond mtu can change +def _get_connectivity_timeout(options): +try: +return int(options.get('connectivityTimeout')) +except: +return None + + def _check_connectivity(connectivity_check_networks, networks, bondings, options, logger): if utils.tobool(options.get('connectivityCheck', True)): logger.debug('Checking connectivity...') -if not clientSeen(int(options.get('connectivityTimeout', - CONNECTIVITY_TIMEOUT_DEFAULT))): +if not clientSeen(_get_connectivity_timeout(options)): logger.info('Connectivity check failed, rolling back') for network in connectivity_check_networks: # If the new added network was created on top of -- To view, visit https://gerrit.ovirt.org/40087 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ibe5e28247bebd91878c2a2c687b2eeb9bfec6586 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ido Barkan ibar...@redhat.com ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: network: clientSeen should have it's default timeout
oVirt Jenkins CI Server has posted comments on this change. Change subject: network: clientSeen should have it's default timeout .. Patch Set 1: Build Started (2/3) - http://jenkins.ovirt.org/job/vdsm_master_network_functional_tests_gerrit/2877/ -- To view, visit https://gerrit.ovirt.org/40087 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibe5e28247bebd91878c2a2c687b2eeb9bfec6586 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ido Barkan ibar...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: network: clientSeen should have it's default timeout
oVirt Jenkins CI Server has posted comments on this change. Change subject: network: clientSeen should have it's default timeout .. Patch Set 1: Build Started (3/3) - http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/18117/ -- To view, visit https://gerrit.ovirt.org/40087 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibe5e28247bebd91878c2a2c687b2eeb9bfec6586 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ido Barkan ibar...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Ondřej Svoboda osvob...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: network: clientSeen should have it's default timeout
oVirt Jenkins CI Server has posted comments on this change. Change subject: network: clientSeen should have it's default timeout .. Patch Set 1: Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/18117/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/17945/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_network_functional_tests_gerrit/2877/ : There was an infra issue, please contact in...@ovirt.org -- To view, visit https://gerrit.ovirt.org/40087 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibe5e28247bebd91878c2a2c687b2eeb9bfec6586 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ido Barkan ibar...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Ondřej Svoboda osvob...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches