Dan Kenigsberg has posted comments on this change.
Change subject: Reduce calling to ethtool.
......................................................................
Patch Set 3: I would prefer that you didn't submit this
(2 inline comments)
minor comments - I like the spirit.
....................................................
File lib/vdsm/netinfo.py
Line 180: return prefix2netmask(netmask)
Line 181:
Line 182:
Line 183: def getIpInfo(dev):
Line 184: dev_info_list = ethtool.get_interfaces_info(dev.encode('utf8'))
nit: dev_info_list has always one element. I'd put THAT element in a devInfo
variable.
Line 185: addr = dev_info_list[0].ipv4_address
Line 186: netmask = dev_info_list[0].ipv4_netmask
Line 187: ipv6addrs = dev_info_list[0].get_ipv6_addresses()
Line 188:
Line 356:
Line 357:
Line 358: def _bridgeinfo(bridge, routes, ipv6routes):
Line 359: ipv4addr, ipv4netmask, ipv6addrs = getIpInfo(bridge)
Line 360: return {'ports': ports(bridge),
when you put this like that, it would make sense to order the keys consistently
(here and in nic/bond/vlan). it would make the next step clearer and more
obvious: the code should be unified.
Line 361: 'stp': bridge_stp_state(bridge),
Line 362: 'addr': ipv4addr,
Line 363: 'netmask': ipv4netmask,
Line 364: 'gateway': routes.get(bridge, '0.0.0.0'),
--
To view, visit http://gerrit.ovirt.org/13166
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2c161b112865ef5cd4842fe70f26a3eeab1c3dd
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon <[email protected]>
Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Livnat Peer <[email protected]>
Gerrit-Reviewer: Mark Wu <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
