Change in vdsm[master]: adapt host outbound qos link share values so they would be a...

danken at redhat.com danken at redhat.com
Mon Jan 12 15:37:20 UTC 2015


Dan Kenigsberg has posted comments on this change.

Change subject: adapt host outbound qos link share values so they would be always divisible by 8
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.ovirt.org/#/c/36636/2/tests/qosTests.py
File tests/qosTests.py:

Line 35
Line 36
Line 37
Line 38
Line 39
this change seems to be needed back in the previous patch.


http://gerrit.ovirt.org/#/c/36636/2/vdsm/network/tc/cls.py
File vdsm/network/tc/cls.py:

Line 26:     complex inputs, e.g. {'ls': ['rate', '400kbps']}"""
Line 27:     command = ['class', 'add', 'dev', dev, 'parent', parent,
Line 28:                'classid', classid]
Line 29:     command.append(kind)
Line 30:     _adapt_qos_options_link_share(opts)
I'm still worried about _adapt_qos_options* modifying its argument. Someone may want to add two classes with the same options, and then be surprised that their opts differ by a factor of 8.

Am I mistaken? Even if we are safe, this is not an expected behavior of a function named add().
Line 31:     for key, value in _qos_to_str_dict(opts).items():
Line 32:         if isinstance(value, str):
Line 33:             command += [key, value]
Line 34:         else:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I00a227f017bf54b06af2a5cbecfb6b5b73113031
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ido Barkan <ibarkan at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Ido Barkan <ibarkan at redhat.com>
Gerrit-Reviewer: Lior Vernia <lvernia at redhat.com>
Gerrit-Reviewer: Ondřej Svoboda <osvoboda 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