Change in vdsm[master]: tc: Integrate Host QoS with network addition via setupNetworks

asegurap at redhat.com asegurap at redhat.com
Tue Sep 2 22:30:37 UTC 2014


Antoni Segura Puimedon has posted comments on this change.

Change subject: tc: Integrate Host QoS with network addition via setupNetworks
......................................................................


Patch Set 12:

(4 comments)

http://gerrit.ovirt.org/#/c/30467/12/vdsm/network/configurators/qos.py
File vdsm/network/configurators/qos.py:

Line 28:                                   StrictVersion('3.5.0')) else 'sfq'
Line 29: 
Line 30: 
Line 31: def configure_outbound(qosOutbound, top_device):
Line 32:     """Adds the qosOutbound configuration to the backing device (be it bond
> "backing device" is a better term than log_dev. Let's use the first instead
Done
Line 33:     or nic). Adds a class and filter for default traffic if necessary"""
Line 34:     description = top_device.description
Line 35:     vlan_tag = description.get('vlan_tag')
Line 36:     device = description['log_dev']


Line 36:     device = description['log_dev']
Line 37:     qdiscs = list(tc._qdiscs(device))
Line 38:     root_qdisc = _root_qdisc(qdiscs)
Line 39:     class_id = '%x' % (_NON_VLANNED_ID if vlan_tag is None else vlan_tag)
Line 40:     if not root_qdisc or root_qdisc['kind'] != 'hfsc':
> 'hfsc'  strings appears too often, risking a typo. Could you define a const
Done
Line 41:         _fresh_qdisc_conf_out(device, vlan_tag, class_id,
Line 42:                               _qos_to_str_dict(qosOutbound))
Line 43:     else:
Line 44:         _qdisc_conf_out(device, root_qdisc['handle'], vlan_tag, class_id,


http://gerrit.ovirt.org/#/c/30467/12/vdsm/network/models.py
File vdsm/network/models.py:

Line 111:     def __repr__(self):
Line 112:         return 'Nic(%s)' % self.name
Line 113: 
Line 114:     @property
Line 115:     def description(self):
> To me, "description" is a human-readable text. Not a bag of fact extracted 
Done
Line 116:         """Returns a dictionary with information about the underlying hierarchy
Line 117:         of the device"""
Line 118:         return {'log_dev': self.name}
Line 119: 


Line 114:     @property
Line 115:     def description(self):
Line 116:         """Returns a dictionary with information about the underlying hierarchy
Line 117:         of the device"""
Line 118:         return {'log_dev': self.name}
> "log_dev" is too opaque name for the nic or bond underlying beneath the net
Done
Line 119: 
Line 120: 
Line 121: class Vlan(NetDevice):
Line 122:     MAX_ID = 4094


-- 
To view, visit http://gerrit.ovirt.org/30467
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4a5378870a3dac9307a6eef8d9937a9c03a0c819
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: 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