Nir Soffer has posted comments on this change.
Change subject: iproute2 ip link wrapper
......................................................................
Patch Set 8:
(5 comments)
Partial review regarding Python and object design.
Also, I guess this is not the minimal fix for that bug. Can we separate the fix
from the new fancy parser?
....................................................
File lib/vdsm/ipwrapper.py
Line 87: return '%s: %s(%s) %s' % (self.index, self.name, self.type,
Line 88: self.address)
Line 89:
Line 90: @staticmethod
Line 91: def parse(text):
staticmethod in Python is pointless - use module level function.
Line 92: """parses a line of output from "ip -o -d link" into a Link
attribute
Line 93: dictionary and returns it."""
Line 94: attrs = {}
Line 95: attrs['index'], attrs['name'], data = [el.strip() for el in
Line 117: range(0, len(tokens)-1, 2))
Line 118: return attrs
Line 119:
Line 120: @classmethod
Line 121: def fromText(cls, text, detection=True):
This could be also a regular function.
Line 122: """Creates a Link object from the textual representation from
Line 123: iproute2's ip link show command."""
Line 124: attrs = cls.parse(text)
Line 125: if 'linkType' not in attrs:
Line 129: attrs['name'] = attrs['name'].split('@')[0]
Line 130: return cls(**attrs)
Line 131:
Line 132: @staticmethod
Line 133: def detectType(name):
Why this belong to the Link class?
Line 134: """Returns the LinkType for the specified device."""
Line 135: # TODO: Add support for virtual functions
Line 136: detectedType = None
Line 137: rc, out, _ = execCmd([_ETHTOOL_BINARY.cmd, '-i', name])
....................................................
File lib/vdsm/netinfo.py
Line 78:
Line 79: fakeNics = config.get('vars', 'fake_nics').split(',')
Line 80: for device in deviceLinks:
Line 81: if device.type == LinkType.NIC or (device.type ==
LinkType.DUMMY and
Line 82: _match_name(device.name, fakeNics)):
Can we use smarter object?
if device.isNIC() or (device.isDUMMY() and device.isFake()):
yield device.name
Line 83: yield device.name
Line 84:
Line 85:
Line 86: def nics():
Line 144:
Line 145: def vlans():
Line 146: hidden_vlans = config.get('vars', 'hidden_vlans').split(',')
Line 147: return [link.name for link in getLinks() if link.type ==
LinkType.VLAN and
Line 148: not _match_name(link.name, hidden_vlans)]
If we have a link object, maybe we can clean up this code by making the object
little smarter?
return [link.name for link in getLinks()
if link.isVLAN() and not link.isHidden()]
Line 149:
Line 150:
Line 151: def bridges():
Line 152: return [b.split('/')[-2] for b in iglob('/sys/class/net/*/bridge')
--
To view, visit http://gerrit.ovirt.org/21054
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibeee70574536e838076704e76f86f2777d5db9b0
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon <[email protected]>
Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]>
Gerrit-Reviewer: Assaf Muller <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
