Martin Sivák has posted comments on this change. Change subject: Adding "updateVmPolicy" api ......................................................................
Patch Set 44: (7 comments) I rebased the patches, all tests pass and we will reverify the functionality of course. 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 Done 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? It is written to the metadata section as far as VDSM is concerned. We tested the set/get logic, MoM will then use it so it will be verified there. 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. Done 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 p Done 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 Done Line 3547: Line 3548: return {'status': doneCode} Line 3549: Line 3550: def _createTransientDisk(self, diskParams): http://gerrit.ovirt.org/#/c/27272/44/vdsm_api/Bridge.py File vdsm_api/Bridge.py: Line 363: 'VM_hotUnplugNic': {'ret': 'vmList'}, Line 364: 'VM_mergeStatus': {'ret': 'mergeStatus'}, Line 365: 'VM_migrationCreate': {'ret': VM_migrationCreate_Ret}, Line 366: 'VM_setNumberOfCpus': {'ret': 'vmList'}, Line 367: 'VM_updateVmPolicy': {}, > We do not need empty command_info. Please remove. Done Line 368: 'Volume_copy': {'ret': 'uuid'}, Line 369: 'Volume_create': {'ret': 'uuid'}, Line 370: 'Volume_delete': {'ret': 'uuid'}, Line 371: 'Volume_getInfo': {'ret': 'info'}, http://gerrit.ovirt.org/#/c/27272/44/vdsm_api/vdsmapi-schema.json File vdsm_api/vdsmapi-schema.json: Line 7364: # @vmID: The UUID of the VM Line 7365: # Line 7366: # @vcpuLimit: vcpu limit to set - the value is a percentage representation Line 7367: # of the amount of cpu from the Host that the VM can consume Line 7368: # > Please align in line description of parameters. Done Line 7369: # Returns: Line 7370: # The VM definition, as updated Line 7371: # Line 7372: # Since: 4.15.0 -- 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
