Dan Kenigsberg has posted comments on this change. Change subject: tc: replace mirred filter parsing with a generic filter parser ......................................................................
Patch Set 8: (4 comments) less comments http://gerrit.ovirt.org/#/c/29211/8/tests/tcTests.py File tests/tcTests.py: Line 267: path = os.path.join(dirName, "tc_filter_show.out") Line 268: with open(path) as tc_filter_show: Line 269: data = tc_filter_show.read() Line 270: Line 271: for parsed, correct in izip_longest(tc._filters(None, out=data), thanks. Line 272: filters): Line 273: self.assertEqual(parsed, correct) Line 274: Line 275: http://gerrit.ovirt.org/#/c/29211/8/vdsm/network/tc/__init__.py File vdsm/network/tc/__init__.py: Line 202: if out is None: Line 203: out = module.show(dev, **kwargs) Line 204: Line 205: for line in _parser.linearize(out.splitlines()): Line 206: tokens = _parser.tokenize(line) I find 'tokenize()' misleading. line is not longer text, it has already been tokenized (on whitespace boudaries) by linearize(). tokens = iter(line) is simpler and clearer Line 207: _parser.consume(tokens, 'qdisc', 'class', 'filter') Line 208: yield module.parse(tokens) Line 209: Line 210: Line 207: _parser.consume(tokens, 'qdisc', 'class', 'filter') Line 208: yield module.parse(tokens) Line 209: Line 210: Line 211: _filters = partial(_iterate, tc_filter) # Accepts a 'parent' selector I still do not understand the comment :-( _filters() has (dev, parent, pref) as arguments. Is "parent" special in any way? http://gerrit.ovirt.org/#/c/29211/8/vdsm/network/tc/filter.py File vdsm/network/tc/filter.py: Line 134: _parser.consume(tokens, 'to') Line 135: _parser.consume(tokens, 'device') Line 136: data['target'] = next(tokens)[:-1] Line 137: data['op'] = next(tokens) Line 138: _parser.consume(tokens, 0) # the 0 marks new line Better use the new constant LINE_DELIMITER Line 139: for token in tokens: Line 140: if token in ('index', 'ref', 'bind'): Line 141: data[token] = int(next(tokens)) Line 142: elif token == 0: -- To view, visit http://gerrit.ovirt.org/29211 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I95aa9aaaeb0f8e612734b32a578e46fade7fdd9c 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: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Livnat Peer <[email protected]> Gerrit-Reviewer: [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
