Kobi Ianko has posted comments on this change. Change subject: Adding utility methods and conf for CPU limit MOM integration ......................................................................
Patch Set 35: (6 comments) comments fixed 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? Done 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. Done 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. Done 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 followin Done 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 Attribut If nothing throws exception here, I will remove the try/except block. the numbers are not invented, the quota -1 means, do not use a quota, and period 1000 is a valid period, it does not matter which valid value since the quota is -1. 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 Done 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 <k...@redhat.com> Gerrit-Reviewer: Adam Litke <ali...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: David Caro <dcaro...@redhat.com> Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com> Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com> Gerrit-Reviewer: Kobi Ianko <k...@redhat.com> Gerrit-Reviewer: Martin Sivák <msi...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
