Change in vdsm[master]: Adding utility methods and conf for CPU limit MOM integration

kobi at redhat.com kobi at redhat.com
Mon May 26 08:48:50 UTC 2014


Kobi Ianko has posted comments on this change.

Change subject: Adding utility methods and conf for CPU limit MOM integration
......................................................................


Patch Set 20:

(4 comments)

http://gerrit.ovirt.org/#/c/27258/20/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 4389: 
Line 4390:     def _getUserCpuTuneInfo(self):
Line 4391:         ret = {}
Line 4392:         try:
Line 4393:             domain = self._connection.lookupByUUIDString(self.id)
> the _dom object is a reference to libvirt. If it's ever becomes stale, it's
Done
Line 4394: 
Line 4395:             if domain:
Line 4396:                 vcpuLimitUri = 'http://ovirt.org/param/vcpu_limit'
Line 4397:                 metadataCpuLimit = domain.metadata(2, vcpuLimitUri, 0)


Line 4392:         try:
Line 4393:             domain = self._connection.lookupByUUIDString(self.id)
Line 4394: 
Line 4395:             if domain:
Line 4396:                 vcpuLimitUri = 'http://ovirt.org/param/vcpu_limit'
> what is this URI? I see nothing there. Is it documented anywhere?
will add documentation
Line 4397:                 metadataCpuLimit = domain.metadata(2, vcpuLimitUri, 0)
Line 4398: 
Line 4399:                 if metadataCpuLimit:
Line 4400:                     pattern = "^<vcpulimit>(.*)</vcpulimit>"


Line 4396:                 vcpuLimitUri = 'http://ovirt.org/param/vcpu_limit'
Line 4397:                 metadataCpuLimit = domain.metadata(2, vcpuLimitUri, 0)
Line 4398: 
Line 4399:                 if metadataCpuLimit:
Line 4400:                     pattern = "^<vcpulimit>(.*)</vcpulimit>"
> The string looks like an xml element. If it is a real xml, it should confor
Since the input is hard coded and known I feel we can skip all xml validations
Line 4401:                     metadataCpuLimit = re.search(pattern,
Line 4402:                                                  metadataCpuLimit, re.M)
Line 4403:                     ret['vcpu_user_limit'] = metadataCpuLimit.group(1)
Line 4404:                 else:


Line 4475:     def setCpuTuneQuota(self, quota):
Line 4476: 
Line 4477:         if self._dom is None:
Line 4478:             return self.reportError(entity='vcpu quota')
Line 4479:         try:
> I see no fix
you are right, forgot to extract the int, now it is fixed
Line 4480:             quota = int(quota)
Line 4481:             self._dom.setSchedulerParameters({'vcpu_quota': quota})
Line 4482:             return {'status': doneCode}
Line 4483:         except ValueError:


-- 
To view, visit http://gerrit.ovirt.org/27258
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic502d9a4a976cd76bb6042bbb51f6cd281199631
Gerrit-PatchSet: 20
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Kobi Ianko <kobi at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: David Caro <dcaroest at redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfediuck 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: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list