Dan Kenigsberg has posted comments on this change.
Change subject: Hidden bonds[2/2]: Refactored and updated nics implementation.
......................................................................
Patch Set 5: I would prefer that you didn't submit this
(3 inline comments)
Partial review, sorry.
....................................................
File lib/vdsm/netinfo.py
Line 77: """
Line 78: res = []
Line 79: hidden_bonds = config.get('vars', 'hidden_bonds').split(',')
Line 80:
Line 81: def isVisibleNic(device):
For symmetry, name ishidden... And reverse logic when needed.
Introducing a new term, "visible", is going to confuse me.
Line 82: """
Line 83: Returns boolean given the device name stating
Line 84: if the device is a not hidden, so visibile nic.
Line 85: """
Line 88:
Line 89: def isHiddenBond(device):
Line 90: return _match_name(device, hidden_bonds)
Line 91:
Line 92: def isNotEnslavedHidBond(device):
It's not too long, but I'd rather loose the negation. Use python's "not" when
needed.
Line 93: """
Line 94: Returns boolean stating if nic is not enslaved
Line 95: to an hidden bond.
Line 96: """
Line 100: if device in slaves(bond):
Line 101: return False
Line 102: return True
Line 103:
Line 104: def predicate(device):
I do not understand the function name
Line 105: """Returns boolean stating if a device should be managed by
vdsm."""
Line 106: if isFakeNic(device):
Line 107: return True
Line 108: elif (isVisibleNic(device) and
--
To view, visit http://gerrit.ovirt.org/16239
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieab80a3ca6b243ade75e490b7004bebe96248140
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Giuseppe Vallarelli <[email protected]>
Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Giuseppe Vallarelli <[email protected]>
Gerrit-Reviewer: Livnat Peer <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
