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