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 <[email protected]>
Gerrit-Reviewer: Adam Litke <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: David Caro <[email protected]>
Gerrit-Reviewer: Doron Fediuck <[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

Reply via email to