Dan Kenigsberg has posted comments on this change. Change subject: tc: add qdisc parsing for some hfsc, ingress, pfifo_fast, etc ......................................................................
Patch Set 4: Code-Review-1 (3 comments) http://gerrit.ovirt.org/#/c/30048/4/vdsm/network/tc/_parser.py File vdsm/network/tc/_parser.py: Line 37: return int(size[:-1]) Line 38: Line 39: Line 40: def parse_time(tokens): Line 41: """Returns a numerical µs representation of the textual size in tokens""" does this worth utf8? (I do not mind, it's cute) Line 42: size = next(tokens) Line 43: if size[-2:] == 'ms': Line 44: return float(size[:-2]) * 10 ** 3 Line 45: elif size[-2:] == 'us': http://gerrit.ovirt.org/#/c/30048/4/vdsm/network/tc/qdisc.py File vdsm/network/tc/qdisc.py: Line 34: Line 35: def parse(tokens): Line 36: """Takes a token generator""" Line 37: kind = next(tokens) Line 38: base_parser = _spec[_BASE_PARSER] Is this separation to base/spec a well-known tc qdisc definition? I could certainly use an explanation in the docstring Line 39: spec_parser = _spec.get(kind, ()) Line 40: data = {'kind': kind, 'handle': next(tokens)} Line 41: for token in tokens: Line 42: if kind not in data and token in base_parser: Line 64: return [int(next(tokens)) for _ in range(_TC_PRIO_MAX)] Line 65: Line 66: Line 67: _spec = { Line 68: _BASE_PARSER: { Does _BASE_PARSER really belong here? Wouldn't it be simpler and nicer if it was a standalone dictionary? This would alleviate the need to define a magical fake "kind". I personally prefer to separate code and data, unless the declarative approach is significantly more concise and robust. A not-so-long elif could replace this dictionary. Line 69: 'dev': _parser.parse_str, Line 70: 'parent': _parser.parse_str, Line 71: 'refcnt': _parser.parse_int, Line 72: 'root': _parser.parse_true, -- To view, visit http://gerrit.ovirt.org/30048 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I039dc7594a261ccb32f78c0b050b28fecac417ee Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: 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
