Change in vdsm[master]: hooks:checkips: add checkips hook

danken at redhat.com danken at redhat.com
Thu Mar 31 11:56:46 UTC 2016


Dan Kenigsberg has posted comments on this change.

Change subject: hooks:checkips: add checkips hook
......................................................................


Patch Set 9: Code-Review-1

(6 comments)

https://gerrit.ovirt.org/#/c/54102/9/vdsm_hooks/checkips/after_get_stats.py
File vdsm_hooks/checkips/after_get_stats.py:

PS9, Line 61: sate
typo: state


Line 61: def update_networks_sate(stats_json, networks, stats_dir):
Line 62:     for net, net_attrs in six.iteritems(networks):
Line 63:         ping_addresses = checkips_utils.get_ping_addresses(net_attrs)
Line 64:         if ping_addresses and not _is_network_accessible(net, stats_dir):
Line 65:             net_if = None
if 'nic' in net_attr:
  ....
elif 'bonding':
  ....
else
  ....

would be simpler than this for loop, in my opinion.
Line 66:             for net_type in ('nic', 'bonding'):
Line 67:                 if net_type in net_attrs:
Line 68:                     net_if = net_attrs[net_type]
Line 69:                     break


PS9, Line 81: checkipv4
better call this eth0_ipv4, so it is clear it's a nic


PS9, Line 88: checkipv6
bond_ipv6 is a clearer name for this one.


PS9, Line 93: test_network
check_fqdn, for  consistency


PS9, Line 95: test_nic
eth_fqdn, maybe?


-- 
To view, visit https://gerrit.ovirt.org/54102
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I53cec37310f0f1844d6fe244419fd8c10e9b7ebb
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Artyom Lukianov <alukiano at redhat.com>
Gerrit-Reviewer: Artyom Lukianov <alukiano at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Edward Haas <edwardh at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list