Douglas Schilling Landgraf has posted comments on this change.

Change subject: engine_page: use vdsm to detect mgmt interface
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.ovirt.org/#/c/24417/2/src/engine_page.py
File src/engine_page.py:

Line 48: 
Line 49:     mgmtIface = []
Line 50:     if 'ovirtmgmt' in nets:
Line 51:         if 'bridge' in nets['ovirtmgmt']:
Line 52:             mgmtIface = [nets['ovirtmgmt'].get('iface', 
nets['ovirtmgmt']['bridge'])]
> the if and else are redundant.
Toni, didn't work for cases that we don't have ovirtmgmt that's why I used the 
conditional.

nets['rhevm'].get('iface', nets['rhevm']['bridge'])
Traceback (most recent call last):
KeyError: 'bridge'

 nets['rhevm']
{'qosInbound': '', 'iface': u'bond0.10', 'qosOutbound': '', 'bridged': False}
Line 53:         else:
Line 54:             mgmtIface = [nets['ovirtmgmt'].get('iface', 
nets['ovirtmgmt'])]
Line 55: 
Line 56:     if 'rhevm' in nets:


Line 48: 
Line 49:     mgmtIface = []
Line 50:     if 'ovirtmgmt' in nets:
Line 51:         if 'bridge' in nets['ovirtmgmt']:
Line 52:             mgmtIface = [nets['ovirtmgmt'].get('iface', 
nets['ovirtmgmt']['bridge'])]
> can't you say here just:
make sense
Line 53:         else:
Line 54:             mgmtIface = [nets['ovirtmgmt'].get('iface', 
nets['ovirtmgmt'])]
Line 55: 
Line 56:     if 'rhevm' in nets:


Line 54:             mgmtIface = [nets['ovirtmgmt'].get('iface', 
nets['ovirtmgmt'])]
Line 55: 
Line 56:     if 'rhevm' in nets:
Line 57:         if 'bridge' in nets['rhevm']:
Line 58:             mgmtIface = [nets['rhevm'].get('iface', 
nets['rhevm']['bridge'])]
> same as above
yep
Line 59:         else:
Line 60:             mgmtIface = [nets['rhevm'].get('iface', nets['rhevm'])]
Line 61: 
Line 62:     mgmt = Management()


-- 
To view, visit http://gerrit.ovirt.org/24417
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9e1b69c0f465f1e38779e451777cae67bd4cde40
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-node-plugin-vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf <[email protected]>
Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]>
Gerrit-Reviewer: Douglas Schilling Landgraf <[email protected]>
Gerrit-Reviewer: Fabian Deutsch <[email protected]>
Gerrit-Reviewer: Joey Boggs <[email protected]>
Gerrit-Reviewer: Ryan Barry <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to