Adam Litke has posted comments on this change. Change subject: Adding support in CPU monitoring ......................................................................
Patch Set 3: Code-Review-1 (6 comments) Please address a few more small comments. http://gerrit.ovirt.org/#/c/28815/3/doc/Makefile.am File doc/Makefile.am: Line 28: balloon-qos.rules \ Line 29: balloon.rules \ Line 30: ksm.rules \ Line 31: mom-balloon+ksm.conf \ Line 32: mom-balloon+ksm+cpu.conf \ You can drop this change. Line 33: mom-balloon.conf \ Line 34: $(NULL) Line 35: Line 36: clean-local: \ http://gerrit.ovirt.org/#/c/28815/3/doc/mom-balloon%2Bksm%2Bcpu.conf File doc/mom-balloon+ksm+cpu.conf: Line 1: [main] It's not necessary to have an example conf for every permutation of collectors. You can drop this from the patch. Line 2: # The wake up frequency of the main daemon (in seconds) Line 3: main-loop-interval: 5 Line 4: Line 5: # The data collection interval for host statistics (in seconds) http://gerrit.ovirt.org/#/c/28815/3/mom.spec.in File mom.spec.in: Line 59: Line 60: install -d -m 0755 "%{buildroot}/%{_initrddir}" Line 61: install -m 0755 contrib/momd.init "%{buildroot}/%{_initrddir}/momd" Line 62: install -d -m 0755 "%{buildroot}/%{_sysconfdir}" Line 63: install -m 0644 doc/mom-balloon+ksm+cpu.conf "%{buildroot}/%{_sysconfdir}/momd.conf" Since this is all pretty oVirt specific. let's keep the old configuration as the default (just ballooning and ksm) for momd deployments. Line 64: Line 65: %check Line 66: make check %{?_smp_mflags} Line 67: http://gerrit.ovirt.org/#/c/28815/3/mom/Collectors/GuestCpuTune.py File mom/Collectors/GuestCpuTune.py: Line 13: Line 14: from mom.Collectors.Collector import * Line 15: class GuestCpuTune(Collector): Line 16: """ Line 17: This Collector uses hypervisor interface to collect guest cpu info You forgot to document the meaning of each field in the docstring here. Please add it. Line 18: """ Line 19: def getFields(self=None): Line 20: return set(['vcpu_quota', 'vcpu_period', 'vcpu_user_limit', 'vcpu_count']) Line 21: http://gerrit.ovirt.org/#/c/28815/3/mom/HypervisorInterfaces/libvirtInterface.py File mom/HypervisorInterfaces/libvirtInterface.py: Line 252: # Get the user selection for vcpuLimit from the metadata Line 253: metadataCpuLimit = None Line 254: try: Line 255: metadataCpuLimit = domain.metadata( Line 256: libvirt.VIR_DOMAIN_METADATA_ELEMENT, 'http://ovirt.org/vm/tune/1.0', 0) the url should be a constant defined at the top of the file. Line 257: except libvirt.libvirtError as e: Line 258: if e.get_error_code() != libvirt.VIR_ERR_NO_DOMAIN_METADATA: Line 259: self.logger.error("Failed to retrieve QoS metadata") Line 260: Line 266: else: Line 267: ret['vcpu_user_limit'] = 100 Line 268: Line 269: # Retrieve the current cpu tuning params Line 270: # self.logger.error(domain.schedulerParameters()) Please remove this. Line 271: ret.update(domain.schedulerParameters()) Line 272: Line 273: if ret['vcpu_quota'] == None: Line 274: ret['vcpu_quota'] = 0 -- To view, visit http://gerrit.ovirt.org/28815 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I66aa9723d3cfc6fc1c1d36a3bf42086abd010f26 Gerrit-PatchSet: 3 Gerrit-Project: mom Gerrit-Branch: master Gerrit-Owner: Kobi Ianko <[email protected]> Gerrit-Reviewer: Adam Litke <[email protected]> Gerrit-Reviewer: Kobi Ianko <[email protected]> Gerrit-Reviewer: Martin Sivák <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
