Francesco Romani has posted comments on this change. Change subject: caps: refactor cpuinfo parsing ......................................................................
Patch Set 4: Code-Review+1 (3 comments) I think is good enough. Added suggestions for future improvements. https://gerrit.ovirt.org/#/c/51523/4/vdsm/caps.py File vdsm/caps.py: Line 92: Line 93: class CpuInfo(object): Line 94: def __init__(self, cpuinfo='/proc/cpuinfo'): Line 95: """Parse /proc/cpuinfo""" Line 96: self.fields = {} do we need this public? Line 97: Line 98: if cpuarch.is_ppc(cpuarch.real()): Line 99: self.fields['flags'] = ['powernv'] Line 100: Line 110: Line 111: if key == 'cpu MHz': Line 112: self.fields['frequency'] = value Line 113: Line 114: if key == 'clock': elif? I believe one key is expected on PPC and one on x86_64, right? If so it would be nice to document this - in a future patch. Line 115: self.fields['frequency'] = value[:-3] Line 116: Line 117: if key == 'model name' or key == 'cpu': Line 118: self.fields['model'] = value Line 114: if key == 'clock': Line 115: self.fields['frequency'] = value[:-3] Line 116: Line 117: if key == 'model name' or key == 'cpu': Line 118: self.fields['model'] = value same Line 119: Line 120: if len(self.fields) == 3: Line 121: break Line 122: -- To view, visit https://gerrit.ovirt.org/51523 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib705bba4789bd1938bf26ae387af72b079020ce7 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik <[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: Milan Zamazal <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: gerrit-hooks <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
