Dan Kenigsberg has posted comments on this change.
Change subject: Change source of CPU sockets/cores to "/sys" and add cpuThreads.
......................................................................
Patch Set 4: I would prefer that you didn't submit this
(7 inline comments)
only a partial review.
I'll need more introduction about what information you are trying to extract
from /topology.
....................................................
Commit Message
Line 15: systems. Moreover adds the cpuThreads to getVdsCapabilities.
so why keep part of the code parsing /proc/cpuinfo?
....................................................
File vdsm/caps.py
Line 64: def __init__(self, cpuinfo='/proc/cpuinfo', \
no need for trailing backslash here (and elsewhere inside parenthesis)
Line 82: r = re.compile('^cpu[0-9]+')
as Gal mentioned, it would save some time to compile on import time.
Line 95: # Cleaning the topology due to error.
do you ever expect /sys to be missing? we use it in so many places in the code.
If it gives more information, and exists on all of our supported platforms,
let's collect the information only from /sys/dev/system/cpu.
Line 101: def siblings(self, cputopology, cpudir, siblingsfile):
I suspect it's only a helper function (so name it _sibling). Do you really need
the siblings' identity? or wouldn't the count be enough?
Line 124:
self.sockets()
this multiplication assumes that all sockets are alike. do we want that?
Line 143: return len(set([p.get('physical id', '0') \
why was it important to drop the temporary variable phys_ids? I found that it
has improved readability.
--
To view, visit http://gerrit.ovirt.org/5481
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1619e3d9e042bc801c988f099d3b84922f4e03d3
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Amador Pahim <[email protected]>
Gerrit-Reviewer: Amador Pahim <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Gal Hammer <[email protected]>
Gerrit-Reviewer: Michal Skrivanek <[email protected]>
Gerrit-Reviewer: Shu Ming <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://fedorahosted.org/mailman/listinfo/vdsm-patches
