Francesco Romani has posted comments on this change. Change subject: lib: daemon: cpu affinity support using taskset ......................................................................
Patch Set 1: (4 comments) https://gerrit.ovirt.org/#/c/45738/1/lib/vdsm/config.py.in File lib/vdsm/config.py.in: Line 211: ('cpu_affinity', '', Line 212: 'CPU cores on which VDSM is forced to run. ' Line 213: 'supported format is list of integers from 1 to N; range syntax ' Line 214: 'like start-stop (both included) is supported. ' Line 215: 'Default is no affinity: VDSM is not pinned to any core, so ' > besides the fact that feels too radical, no technical objection :) supporting a single cpu not yet done. Line 216: 'it can run on any cpu core.') Line 217: Line 218: ]), Line 219: https://gerrit.ovirt.org/#/c/45738/1/lib/vdsm/utils.py File lib/vdsm/utils.py: Line 1295: yield int(item) Line 1296: Line 1297: Line 1298: def cpu_mask_to_bits(cpus): Line 1299: mask = 0 > to be a bit less C-ish, 'index' sounds nicer ;) (applies also for above) Done Line 1300: if not cpus: Line 1301: return CPU_ANY Line 1302: Line 1303: for idx in _expand_cpu_list(cpus): Line 1300: if not cpus: Line 1301: return CPU_ANY Line 1302: Line 1303: for idx in _expand_cpu_list(cpus): Line 1304: mask = _enable_cpu(mask, idx) > This feels hard to read to me - what about returning only the shift and doi inlined the function https://gerrit.ovirt.org/#/c/45738/1/tests/utilsTests.py File tests/utilsTests.py: Line 963: ['0,-'], Line 964: ['-3'], Line 965: ['4-'], Line 966: [','], Line 967: [',,'] > add '-' as a test case (for peace of mind ;) ) Done Line 968: ]) Line 969: def test_invalid_input(self, cpus): Line 970: self.assertRaises(ValueError, Line 971: utils.cpu_mask_to_bits, -- 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: 1 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: 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: [email protected] Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
