Change in vdsm[master]: tc: Integrate Host QoS with network deletion via setupNetworks
danken at redhat.com
danken at redhat.com
Tue Sep 9 23:52:21 UTC 2014
Dan Kenigsberg has posted comments on this change.
Change subject: tc: Integrate Host QoS with network deletion via setupNetworks
......................................................................
Patch Set 6:
(4 comments)
http://gerrit.ovirt.org/#/c/31440/6/vdsm/network/configurators/qos.py
File vdsm/network/configurators/qos.py:
Line 60: except tc.TrafficControlException as tce:
Line 61: if tce.errCode != errno.EINVAL: # no filter exists
Line 62: raise
Line 63:
Line 64: qdiscs = list(tc._qdiscs(device))
Calling list() is redundant; I'm not judging the intermediate variable.
Line 65: root_qdisc_handle = _root_qdisc(qdiscs)['handle']
Line 66: try:
Line 67: tc.cls.delete(device, classid=root_qdisc_handle + class_id)
Line 68: except tc.TrafficControlException as tce:
Line 70: raise
Line 71:
Line 72: if _no_used_classes(device, parent=root_qdisc_handle):
Line 73: tc._qdisc_del(device)
Line 74: tc._qdisc_del(device, kind='ingress')
pardon my lapse, but where did we add an ingress qdisc?
Line 75:
Line 76:
Line 77: def _no_used_classes(device, parent):
Line 78: """Returns true iff there's no traffic classes in the device, ignoring the
Line 73: tc._qdisc_del(device)
Line 74: tc._qdisc_del(device, kind='ingress')
Line 75:
Line 76:
Line 77: def _no_used_classes(device, parent):
negation in function names tends to confuse me. Can you find a positive name? Maybe _uses_classes()?
The "parent" arg is not well defined, and probably not independent of "device"... "parent" has to be root_qdisc_handle of the "device", isn't it? it's passed as an arg only as a short cut? This should be documented, or expressed by another arg name.
Line 78: """Returns true iff there's no traffic classes in the device, ignoring the
Line 79: root class and a default unused class"""
Line 80: classes = list(tc._classes(device, parent=parent))
Line 81: classes = [cls for cls in classes if not cls.get('root')]
Line 76:
Line 77: def _no_used_classes(device, parent):
Line 78: """Returns true iff there's no traffic classes in the device, ignoring the
Line 79: root class and a default unused class"""
Line 80: classes = list(tc._classes(device, parent=parent))
Calling list() is redundant
Line 81: classes = [cls for cls in classes if not cls.get('root')]
Line 82: return (not classes or
Line 83: (len(classes) == 1 and not netinfo.ifaceUsed(device) and
Line 84: classes[0]['handle'] == parent + _DEFAULT_CLASSID))
--
To view, visit http://gerrit.ovirt.org/31440
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I28a3676af1553052e7cdea515faa29e847b7d3c0
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list