Kobi Ianko has posted comments on this change. Change subject: Adding API methods for CPU limit MOM integration ......................................................................
Patch Set 1: (4 comments) 3 out of 4 comments where fixed. need info on 1 comment http://gerrit.ovirt.org/#/c/28462/1/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 4493: return {'status': doneCode} Line 4494: Line 4495: def setCpuTuneQuota(self, quota): Line 4496: Line 4497: if self._dom is None: > Why do you have this check? it can happen only if setCpuTuneQuota is calle Done Line 4498: return self._reportError(entity='vcpu quota') Line 4499: Line 4500: quota = int(quota) Line 4501: try: Line 4499: Line 4500: quota = int(quota) Line 4501: try: Line 4502: self._dom.setSchedulerParameters({'vcpu_quota': quota}) Line 4503: return {'status': doneCode} > Keeping try-block as short as possible is a good practice. Please call retu Done Line 4504: except libvirt.libvirtError as e: Line 4505: return self._reportError(msg=e.message, entity='vcpu quota') Line 4506: Line 4507: def setCpuTunePeriod(self, period): Line 4518: entity='vcpu period') Line 4519: except libvirt.libvirtError as e: Line 4520: return self._reportError(msg=e.message, entity='vcpu period') Line 4521: Line 4522: def _reportError(self, key='Err', msg=None, entity=None): > please be kind to your reviewer, and separate this refactoring from the add can you elaborate, what exactly should be done with this function? Line 4523: self.log.error("Set new " + entity + " failed", exc_info=True) Line 4524: if msg is None: Line 4525: error = errCode[key] Line 4526: else: Line 4519: except libvirt.libvirtError as e: Line 4520: return self._reportError(msg=e.message, entity='vcpu period') Line 4521: Line 4522: def _reportError(self, key='Err', msg=None, entity=None): Line 4523: self.log.error("Set new " + entity + " failed", exc_info=True) > this explodes if "entity" has its default value of None. Done Line 4524: if msg is None: Line 4525: error = errCode[key] Line 4526: else: Line 4527: error = {'status': {'code': errCode[key] -- To view, visit http://gerrit.ovirt.org/28462 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia78529b736ec0c841d232ba8aa1434bd0d0e8e08 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Kobi Ianko <k...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com> Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com> Gerrit-Reviewer: Kobi Ianko <k...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
