Francesco Romani has posted comments on this change. Change subject: sampling: Handle numa nodes with zero memory assigned ......................................................................
Patch Set 2: Code-Review+1 (2 comments) fine with the concept, but one question inside. Hence +1. https://gerrit.ovirt.org/#/c/44121/2/vdsm/virt/sampling.py File vdsm/virt/sampling.py: Line 181: nodeMemSample['memFree'] = memInfo['free'] Line 182: # in case the numa node has zero memory assigned, report the whole Line 183: # memory as used Line 184: nodeMemSample['memPercent'] = 100 Line 185: if memInfo['total'] != 0: here you check against 0 (as number)... Line 186: nodeMemSample['memPercent'] = 100 - \ Line 187: int(100.0 * int(memInfo['free']) / int(memInfo['total'])) Line 188: self.nodesMemSample[nodeIndex] = nodeMemSample Line 189: Line 183: # memory as used Line 184: nodeMemSample['memPercent'] = 100 Line 185: if memInfo['total'] != 0: Line 186: nodeMemSample['memPercent'] = 100 - \ Line 187: int(100.0 * int(memInfo['free']) / int(memInfo['total'])) ...but here you explicitely convert as int(), to make sure it is indeed int. Now, question: can memInfo['total'] be a string? a float? Or will it be an integer unless sky is falling? I'm fine both with and without explicit int() conversions, as long as it is consistent (or documented why it isn't!) Line 188: self.nodesMemSample[nodeIndex] = nodeMemSample Line 189: Line 190: Line 191: class PidCpuSample(object): -- To view, visit https://gerrit.ovirt.org/44121 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6be69e715e9b4a0cc8ded744b0bcc192a466c6c8 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Roman Mohr <[email protected]> Gerrit-Reviewer: Andrej Krejcir <[email protected]> Gerrit-Reviewer: Artyom Lukianov <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Dudi Maroshi <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Roman Mohr <[email protected]> Gerrit-Reviewer: Roy Golan <[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
