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

danken at redhat.com danken at redhat.com
Sun Jun 8 16:07:01 UTC 2014


Dan Kenigsberg has posted comments on this change.

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


Patch Set 35: Code-Review-1

(6 comments)

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

Line 30: import tempfile
Line 31: import threading
Line 32: import time
Line 33: import xml.dom.minidom
Line 34: import re
is it used?
Line 35: 
Line 36: # 3rd party libs imports
Line 37: import libvirt
Line 38: 


Line 267:     def _sampleCpuTune(self):
Line 268:         infos = self._dom.schedulerParameters()
Line 269:         infos['vcpuCount'] = self._dom.vcpusFlags(libvirt.VIR_DOMAIN_MEM_CURRENT)
Line 270: 
Line 271:         vcpuLimitUri = 'http://ovirt.org/vm/tune/1.0'
This string should be defined as a CONSTANT. Maybe in the module level.
Line 272:         metadataCpuLimit = self._dom.metadata(2, vcpuLimitUri, 0)
Line 273:         if metadataCpuLimit:
Line 274:             metadataCpuLimitXML = _domParseStr(metadataCpuLimit)
Line 275:             nodeList = \


Line 268:         infos = self._dom.schedulerParameters()
Line 269:         infos['vcpuCount'] = self._dom.vcpusFlags(libvirt.VIR_DOMAIN_MEM_CURRENT)
Line 270: 
Line 271:         vcpuLimitUri = 'http://ovirt.org/vm/tune/1.0'
Line 272:         metadataCpuLimit = self._dom.metadata(2, vcpuLimitUri, 0)
Use libvirt.VIR_DOMAIN_METADATA_ELEMENT constant instead on magic 2.
Line 273:         if metadataCpuLimit:
Line 274:             metadataCpuLimitXML = _domParseStr(metadataCpuLimit)
Line 275:             nodeList = \
Line 276:                 metadataCpuLimitXML.getElementsByTagName('vcpulimit')


Line 273:         if metadataCpuLimit:
Line 274:             metadataCpuLimitXML = _domParseStr(metadataCpuLimit)
Line 275:             nodeList = \
Line 276:                 metadataCpuLimitXML.getElementsByTagName('vcpulimit')
Line 277:             value = nodeList[0].childNodes[0].data
if metadataCpuLimit is None, value is not going to be set, and the following statement is going to explode.
Line 278: 
Line 279:         infos['vcpuLimit'] = value
Line 280:         return infos
Line 281: 


Line 342:                 ret['vcpu_period'] = 1000
Line 343:             else:
Line 344:                 ret['vcpu_period'] = eInfo['vcpu_period']
Line 345: 
Line 346:         except (AttributeError, libvirt.libvirtError):
nothing in the try block can raise libvirtError; I cannot find any AttributeError either.

I believe that this is my forth request not report invented values with we do not know the vcpu_period.
Line 347:             # Metadata is not set, set the defaults
Line 348:             self.log.debug('Domain Metadata is not set', exc_info=True)
Line 349:             ret['vcpu_quota'] = -1
Line 350:             ret['vcpu_period'] = 1000


Line 2694:                 stats['network'] = decStats[var]
Line 2695:             elif var == 'balloonInfo':
Line 2696:                 stats['balloonInfo'] = decStats[var]
Line 2697:             elif var == 'vcpu_quota':
Line 2698:                 stats['vcpu_quota'] = decStats[var]
non-dictionary elements do not need explicit handling here
Line 2699:             elif var == 'vcpu_period':
Line 2700:                 stats['vcpu_period'] = decStats[var]
Line 2701:             elif var == 'cpu_count':
Line 2702:                 stats['cpu_count'] = decStats[var]


-- 
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: 35
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