Yaniv Bronhaim has posted comments on this change.

Change subject: scale: limit cpu usage using cpu-affinity
......................................................................


Patch Set 15:

(3 comments)

added few replies - please do the change and work with a wrapper, it won't 
require the cpopen dependency which I much prefer

https://gerrit.ovirt.org/#/c/45738/15/tests/tasksetTests.py
File tests/tasksetTests.py:

Line 68:         self.assertEqual(taskset.get(self.proc.pid), cpu_list)
Line 69: 
Line 70:     def test_set_from_child(self):
Line 71: 
Line 72:         cpu_list = ['0']
> See bellow the 2 tests for bad input, I think they are enough. There is no 
oh, not a fail cases, only test once the option of pinning the process for more 
than one cpu, like ['0','4'] - is it possible? I'm curious if it works..
Line 73: 
Line 74:         def _run_helper():
Line 75:             # avoid race on startup: first do taskset, then notify
Line 76:             taskset.set(os.getpid(), cpu_list)


https://gerrit.ovirt.org/#/c/45738/15/vdsm.spec.in
File vdsm.spec.in:

Line 120: Requires: iproute >= 3.10.0
Line 121: Requires: python-netaddr
Line 122: Requires: python-inotify
Line 123: Requires: python-argparse
Line 124: Requires: python-cpopen >= 1.4
> Since this add new feature, 1.3.3 (bug fix release) does not seems appropri
until we don't have build for that better to use TBD
Line 125: Requires: python-ioprocess >= 0.14
Line 126: Requires: python-pthreading >= 0.1.3-3
Line 127: Requires: python-six
Line 128: Requires: python-requests


https://gerrit.ovirt.org/#/c/45738/15/vdsm/vdsm
File vdsm/vdsm:

Line 255:                          "(stderr: %s). Verify sudoer rules 
configuration" %
Line 256:                          (stderr))
Line 257: 
Line 258: 
Line 259: def _setCpuAffinity():
> two underscores (__) sign global function? Never heard of this convention. 
you can do it in separate patch, but for now keep the file standards ... two 
underscores for private global functions
Line 260:     cpu_affinity = config.get('vars', 'cpu_affinity')
Line 261:     if cpu_affinity:
Line 262:         cpu_list = [cpu.strip() for cpu in cpu_affinity.split(',')]
Line 263:         taskset.set(os.getpid(), cpu_list, all_tasks=True)


-- 
To view, visit https://gerrit.ovirt.org/45738
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I3f7f68d65eddb5a21afbc3809ea79cd1dee67984
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <[email protected]>
Gerrit-Reviewer: Adam Litke <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Francesco Romani <[email protected]>
Gerrit-Reviewer: Ido Barkan <[email protected]>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik <[email protected]>
Gerrit-Reviewer: Martin Sivák <[email protected]>
Gerrit-Reviewer: Michal Skrivanek <[email protected]>
Gerrit-Reviewer: Michal Skrivanek <[email protected]>
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: Piotr Kliczewski <[email protected]>
Gerrit-Reviewer: Vinzenz Feenstra <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: Yaniv Kaul <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to