Change in vdsm[master]: network: clientSeen should have it's default timeout

2015-04-27 Thread danken
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

2015-04-21 Thread automation
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

2015-04-21 Thread oVirt Jenkins CI Server
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

2015-04-21 Thread ibarkan
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

2015-04-21 Thread oVirt Jenkins CI Server
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

2015-04-21 Thread oVirt Jenkins CI Server
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

2015-04-21 Thread oVirt Jenkins CI Server
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