Dan Kenigsberg has posted comments on this change.
Change subject: Use 'ip link' to get different kinds of interfaces
......................................................................
Patch Set 11: (4 inline comments)
thanks, Mark. I would love to see my comments implemented, and this change
merged.
....................................................
File lib/vdsm/netinfo.py
Line 49
Line 50
Line 51
Line 52
Line 53
would you please keep a (possibly slow) implementation of these functions?
def nics():
return getInterfaces().nics
as long as historical vds_bootstrap are still supported, there may be
vds_bootstrap from ovirt-3.0 attempting to call nics().
Line 161: # return the speed of devices that are capable of replying
Line 162: try:
Line 163: # operstat() filters out down/disabled nics
Line 164: # virtio is a valid device, but doesn't support speed
Line 165: if operstate(dev) == 'up' and not isvirtio(dev):
see my comment about the nics() function.
Please try to keep commits atomic, in the sense that the code after any single
commit is valid. If you make no changes here, and keep calling the slow nics()
mentioned above, you can still optimize in the following patch.
Line 166: # the device may have been disabled/downed after checking
Line 167: # so we validate the return value as sysfs may return
Line 168: # special values to indicate the device is down/disabled
Line 169: s = int(file('/sys/class/net/%s/speed' % dev).read())
....................................................
File vdsm/neterrors.py
Line 29: ERR_USED_BRIDGE = 28
Line 30: ERR_FAILED_IFUP = 29
Line 31: ERR_LOST_CONNECTION = 10 # noConPeer
Line 32:
Line 33: ERR_FAILED_IPLINK = 50
do you expect clients to care about the such a specific detail? I do not, and I
would like to avoid proliferation of error codes. I do not see how
ERR_FAILED_IPLINK is more helpful to the client than a general "unexpected
exception".
Line 34:
Line 35:
Line 36: class NetInfoError(Exception):
Line 37: def __init__(self, errCode, message):
....................................................
File vdsm_reg/deployUtil.py.in
Line 979: bonding = ''
Line 980: nic = None
Line 981:
Line 982: try:
Line 983: _, bondings, vlans, _ = netinfo.getInterfaces()
I'm not 100% sure, but if we try to add an ovirt-3.2 node to an ovirt-3.3
Engine, this line would explode (as there's no netinfo.getInterfaces in 3.2).
Line 984: if mgtIface in vlans:
Line 985: nic = netinfo.getVlanDevice(mgtIface)
Line 986: vlan = netinfo.getVlanID(mgtIface)
Line 987: else:
--
To view, visit http://gerrit.ovirt.org/13668
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iecc2300963fe1834defb99f5be92b25c0218cf00
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <[email protected]>
Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[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
