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@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