Dan Kenigsberg has posted comments on this change. Change subject: Adding "updateVmPolicy" api ......................................................................
Patch Set 44: Code-Review-1 (5 comments) http://gerrit.ovirt.org/#/c/27272/44/tests/functional/virtTests.py File tests/functional/virtTests.py: Line 408: with RunningVm(self.vdsm, customization) as vm: Line 409: self._waitForStartup(vm, VM_MINIMAL_UPTIME) Line 410: status, msg, stats = self.vdsm.getVmStats(vm) Line 411: self.assertEqual(status, SUCCESS, msg) Line 412: self.vdsm.updateVmPolicy('99999999-aaaa-ffff-bbbb-111111111111', better use customization[vmId] to avoid string duplication Line 413: '50') Line 410: status, msg, stats = self.vdsm.getVmStats(vm) Line 411: self.assertEqual(status, SUCCESS, msg) Line 412: self.vdsm.updateVmPolicy('99999999-aaaa-ffff-bbbb-111111111111', Line 413: '50') Line 414: self.assertEqual(status, SUCCESS, msg) is it possible to verify that the new policy is acted upon? http://gerrit.ovirt.org/#/c/27272/44/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 3526: params['vcpuLimit'] + Line 3527: '</vcpulimit>', 'ovirt', Line 3528: METADATA_VM_TUNE_URI, 0) Line 3529: except libvirt.libvirtError as e: Line 3530: self.log.error("updateVmPolicy failed", exc_info=True) use exception() that does not need an explicit exc_info=True. Line 3531: if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN: Line 3532: return errCode['noVM'] Line 3533: else: Line 3534: return {'status': {'code': errCode['updateVmPolicyErr'] Line 3530: self.log.error("updateVmPolicy failed", exc_info=True) Line 3531: if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN: Line 3532: return errCode['noVM'] Line 3533: else: Line 3534: return {'status': {'code': errCode['updateVmPolicyErr'] how about using the reportError method which you introduced in a previous patch? Line 3535: ['status']['code'], 'message': e.message}} Line 3536: else: Line 3537: del params['vcpuLimit'] Line 3538: Line 3542: if params: Line 3543: unknownParams = params.keys() Line 3544: unknownParamsStr = ", ".join(unknownParams) Line 3545: self.log.warn("updateVmPolicy got unknown parameters: %s", Line 3546: unknownParamsStr) the intermediate variables are unhelpful imo. Placing ", ".join(params.iterkeys()) on the log is simpler and clearer. Line 3547: Line 3548: return {'status': doneCode} Line 3549: Line 3550: def _createTransientDisk(self, diskParams): -- To view, visit http://gerrit.ovirt.org/27272 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9750667c4d20d7589a1797e65d5683692ec02afe Gerrit-PatchSet: 44 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Kobi Ianko <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: David Caro <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Gilad Chaplik <[email protected]> Gerrit-Reviewer: Kobi Ianko <[email protected]> Gerrit-Reviewer: Martin Sivák <[email protected]> Gerrit-Reviewer: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
