Giuseppe Vallarelli has posted comments on this change.
Change subject: Hidden bonds[2/2]: Refactored and updated nics implementation.
......................................................................
Patch Set 5: (3 inline comments)
....................................................
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):
Consider that the alternative you propose would change (by reading your
following comments):
from:
elif isVisibleNic(device) and isNotEnslavedHidBond(device)
to:
elif not isHiddenNic(device) and not isEnslavedHidBond(device)
If I plan to prefix the query function (it returns a bool) with not than I
would rather put the not within the function, than having the not following the
function call in every other place, for me is much easier to reason about. A
docstring has been added for clarity, being a sort of new concept - but it's
the same from a different perspective.
Related matter:
http://c2.com/cgi/wiki?RefactorNegateIf
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):
See answer above.
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):
This function embodies the set of rules to establish if a device should be
managed by vdsm. I named predicate because I'm using it with the ifilter where
every element of an iterable is filered if the predicate holds. Predicate is
the name of the first argument of ifilter.
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
