Change in vdsm[master]: Adding "updateVmPolicy" api
msivak at redhat.com
msivak at redhat.com
Fri Jun 13 08:33:50 UTC 2014
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 <kobi at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: David Caro <dcaroest at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Gilad Chaplik <gchaplik at redhat.com>
Gerrit-Reviewer: Kobi Ianko <kobi at redhat.com>
Gerrit-Reviewer: Martin Sivák <msivak at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list