Edward Haas has posted comments on this change. Change subject: net: Adding the 'link' package with an iface module ......................................................................
Patch Set 4: (3 comments) https://gerrit.ovirt.org/#/c/62827/4/lib/vdsm/network/link/iface.py File lib/vdsm/network/link/iface.py: Line 32: STATE_UP = 'up' Line 33: STATE_DOWN = 'down' Line 34: Line 35: Line 36: def up(dev, admin_blocking=True, link_blocking=False): > please add a doctext for admin_blocking and link_blocking. Done Line 37: if admin_blocking: Line 38: _up_blocking(dev, link_blocking) Line 39: else: Line 40: ipwrapper.linkSet(dev, [STATE_UP]) PS4, Line 47: is_up > is_admin_up? i don't like the name but it would be less confusing. I think 'is_admin_up' is more confusing in this regard. But I can add it and leave this one as an alias. The default is 'admin' state, I will add a doctext for it. Line 71: Line 72: def _is_iface_up(link_flags, check_link): Line 73: iface_up = link_flags & IFF_UP Line 74: if check_link: Line 75: iface_up = iface_up and (link_flags & IFF_RUNNING) > iface_up &= link_flags & IFF_RUNNING You are confusing bitwise operand with a logical one. -- To view, visit https://gerrit.ovirt.org/62827 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icb723b8d893575ef14e71cacb6b6e391a6a84831 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Edward Haas <[email protected]> Gerrit-Reviewer: Edward Haas <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček <[email protected]> Gerrit-Reviewer: gerrit-hooks <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/admin/lists/[email protected]
