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 wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server