Adam Litke has posted comments on this change. Change subject: GuestMemory fields should be optional ......................................................................
Patch Set 8: Code-Review-1 (2 comments) Just a couple of minor things. Please address my recurring comment about swap_usage in the HypervisorInterface on the next round. I'll be unable to merge this patch until that is fixed. Are you planning on updating the sample policies and config files to use the new optional collector and associated policy logic? http://gerrit.ovirt.org/#/c/34528/8/mom/Collectors/GuestMemoryOptional.py File mom/Collectors/GuestMemoryOptional.py: Line 9: def getFields(self=None): Line 10: return set() Line 11: Line 12: def getOptionalFields(self=None): Line 13: return self.hypervisor_iface.getStatsFields() + set(["swap_total", "swap_usage"]) Please add "swap_total" and "swap_usage" to the return value of getStatsFields in vdsmInterface.py and drop it from here. http://gerrit.ovirt.org/#/c/34528/8/mom/Entity.py File mom/Entity.py: Line 106: if x.get(name, None) is not None] Line 107: for row in nonEmptyStats: Line 108: total = total + row[name] Line 109: if (len(nonEmptyStats) == 0): Line 110: return float(0) Should we have a default value for this case like we do for Stat? Zero could be misleading to the policy. Line 111: else: Line 112: return float(total / len(nonEmptyStats)) Line 113: Line 114: def SetVar(self, name, val): -- To view, visit http://gerrit.ovirt.org/34528 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I631056568b5a69e18f6ed02730c41ab59f460aad Gerrit-PatchSet: 8 Gerrit-Project: mom Gerrit-Branch: master Gerrit-Owner: Roy Golan <rgo...@redhat.com> Gerrit-Reviewer: Adam Litke <ali...@redhat.com> Gerrit-Reviewer: Martin Sivák <msi...@redhat.com> Gerrit-Reviewer: Roy Golan <rgo...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
