Antoni Segura Puimedon has posted comments on this change.

Change subject: tc: add qdisc parsing for some hfsc, ingress, pfifo_fast, etc
......................................................................


Patch Set 4:

(2 comments)

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 c
in TC terms, the split is tc_qdisc.c:print_qdisc which then calls in the 
print_opt of the kind of the qdisc (for hfsc that would be 
q_hfsc.c:hfsc_print_opt

Basically after parsing the netlink message TC has a qdisc object set with the 
correct print_qopt (pointing to hfsc_print_opt for example). I tried to mimick 
this with getting the kind and then just using the parser indexed by the kind.

I'll change _BASE_PARSER with _QDISC='_qdisc'
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 i
I guess I was lisping too much in the past. I will separate the base from the 
kinds.

Changing it into an else if will mean having separate branches for each kind. I 
find it more cumbersome to read, but I could yield.
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: 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

Reply via email to