Change in vdsm[master]: Use 'ip link' to get different kinds of interfaces

danken at redhat.com danken at redhat.com
Wed Apr 10 09:18:52 UTC 2013


Dan Kenigsberg has posted comments on this change.

Change subject: Use 'ip link' to get different kinds of interfaces
......................................................................


Patch Set 4: I would prefer that you didn't submit this

(3 inline comments)

....................................................
File lib/vdsm/netinfo.py
Line 51: def _match_nic_name(nic, patterns):
Line 52:     return any(map(lambda p: fnmatch(nic, p), patterns))
Line 53: 
Line 54: 
Line 55: def _parseIpLinkOutput(links):
could you add a unit test for this new function?

p.s. I bet it already exists in N+1 other projects...
Line 56:     interfaces = {'nic': [], 'bond': [], 'vlan': [], 'bridge': [], 'fake': []}
Line 57:     name = link = None
Line 58:     for line in links:
Line 59:         if re.match('^[0-9]+', line):


Line 80:     fake_nics = config.get('vars', 'fake_nics').split(',')
Line 81:     ret, out, err = execCmd([constants.EXT_IPROUTE, '--details',
Line 82:                              'link', 'show'])
Line 83:     interfaces = _parseIpLinkOutput(out)
Line 84:     return ([nic for nic in interfaces['nic']
how about returning a named tuple? getInterfaces().nics is more readable than  getInterfaces()[0].
Line 85:              if not _match_nic_name(nic, hidden_nics)] +
Line 86:             [nic for nic in interfaces['fake']
Line 87:              if _match_nic_name(nic, fake_nics)],
Line 88:             interfaces['bond'],


Line 156:     # return the speed of devices that are capable of replying
Line 157:     try:
Line 158:         # operstat() filters out down/disabled nics
Line 159:         # virtio is a valid device, but doesn't support speed
Line 160:         if operstate(dev) == 'up' and not isvirtio(dev):
nics() was added by http://gerrit.ovirt.org/4320 in intention to deparate non-existing devices from devices with no speed defined (such as vlan bonds and bridges)

Otherwise, we had annoying log entries.

The right solution for this is probably to make sure that we never call speed(bridge) or its like, and I prefer to do it in a separate patch.
Line 161:             # the device may have been disabled/downed after checking
Line 162:             # so we validate the return value as sysfs may return
Line 163:             # special values to indicate the device is down/disabled
Line 164:             s = int(file('/sys/class/net/%s/speed' % dev).read())


--
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: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw at linux.vnet.ibm.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw at linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list