Dan Kenigsberg has posted comments on this change. Change subject: Adding "updateVmPolicy" api ......................................................................
Patch Set 22: Code-Review-1 (5 comments) Please add a functional test for the new verb. http://gerrit.ovirt.org/#/c/27272/22/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 3224: Line 3225: if self.isMigrating(): Line 3226: return errCode['migInProgress'] Line 3227: Line 3228: self.log.debug("Setting VM policy to [vcpuLimit: %s]", we log each API call on entry. Do we need a second line here? Line 3229: params['vcpuLimit']) Line 3230: Line 3231: try: Line 3232: domain = self._connection.lookupByUUIDString(self.id) Line 3228: self.log.debug("Setting VM policy to [vcpuLimit: %s]", Line 3229: params['vcpuLimit']) Line 3230: Line 3231: try: Line 3232: domain = self._connection.lookupByUUIDString(self.id) please use self._dom Line 3233: domain.setMetadata(2, '<vcpulimit>'+params['vcpuLimit'] + Line 3234: '</vcpulimit>', 'ovirt', Line 3235: 'http://ovirt.org/param/vcpu_limit', 0) Line 3236: except libvirt.libvirtError as e: Line 3229: params['vcpuLimit']) Line 3230: Line 3231: try: Line 3232: domain = self._connection.lookupByUUIDString(self.id) Line 3233: domain.setMetadata(2, '<vcpulimit>'+params['vcpuLimit'] + what is the magic number 2? Line 3234: '</vcpulimit>', 'ovirt', Line 3235: 'http://ovirt.org/param/vcpu_limit', 0) Line 3236: except libvirt.libvirtError as e: Line 3237: self.log.error("updateVmPolicy failed", exc_info=True) Line 3239: return errCode['noVM'] Line 3240: return {'status': {'code': errCode['updateVmPolicyErr'] Line 3241: ['status']['code'], 'message': e.message}} Line 3242: Line 3243: domain = self._connection.lookupByUUIDString(self.id) why is this extra lookup? Line 3244: return {'status': doneCode} Line 3245: Line 3246: def _createTransientDisk(self, diskParams): Line 3247: if diskParams.get('shared', None) != DRIVE_SHARED_TYPE.TRANSIENT: Line 4507: return self.reportError(msg='an integer is required for quota', Line 4508: entity='vcpu quota') Line 4509: except libvirt.libvirtError as e: Line 4510: if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN: Line 4511: return self.reportError(key='noVM', entity='vcpu quota') unrelated whitespace distracts reviewers. Line 4512: Line 4513: return self.reportError(msg=e.message, entity='vcpu quota') Line 4514: Line 4515: def setCpuTunePeriod(self, period): -- 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: 22 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: Gilad Chaplik <[email protected]> Gerrit-Reviewer: Kobi Ianko <[email protected]> Gerrit-Reviewer: Martin Sivák <[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
