Martin Polednik has posted comments on this change. Change subject: numa: refactor to single pass caps parse ......................................................................
Patch Set 9: (3 comments) https://gerrit.ovirt.org/#/c/53288/9/lib/vdsm/numa.py File lib/vdsm/numa.py: Line 46: siblings = set() Line 47: online_cpus = [] Line 48: Line 49: caps = ET.fromstring(capabilities) Line 50: cells = caps.find('host').find('.//cells').findall('cell') > I'm a bit torn because I don't like the chained calls, it is a (light) code Done Line 51: Line 52: for cell in cells: Line 53: cell_id = cell.get('id') Line 54: meminfo = memory_by_cell(int(cell_id)) Line 52: for cell in cells: Line 53: cell_id = cell.get('id') Line 54: meminfo = memory_by_cell(int(cell_id)) Line 55: topology[cell_id]['totalMemory'] = meminfo['total'] Line 56: topology[cell_id]['cpus'] = [] > (very) minor: Tried and I have to say that I dislike using that approach in this particular case. Line 57: distances[cell_id] = [] Line 58: Line 59: for cpu in cell.find('cpus').findall('cpu'): Line 60: topology[cell_id]['cpus'].append(int(cpu.get('id'))) Line 55: topology[cell_id]['totalMemory'] = meminfo['total'] Line 56: topology[cell_id]['cpus'] = [] Line 57: distances[cell_id] = [] Line 58: Line 59: for cpu in cell.find('cpus').findall('cpu'): > same comment as per line 50, albeit less important. Done Line 60: topology[cell_id]['cpus'].append(int(cpu.get('id'))) Line 61: if cpu.get('siblings') and cpu.get('socket_id'): Line 62: online_cpus.append(cpu.get('id')) Line 63: sockets.add(cpu.get('socket_id')) -- To view, visit https://gerrit.ovirt.org/53288 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie81b5b76523070ec9fd7a6ffe902ffa25da9e3a6 Gerrit-PatchSet: 9 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: gerrit-hooks <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
