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

danken at redhat.com danken at redhat.com
Thu Jun 12 23:39:34 UTC 2014


Dan Kenigsberg has posted comments on this change.

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


Patch Set 57: Code-Review-1

(4 comments)

Minor naming issues remain.

http://gerrit.ovirt.org/#/c/27258/57/tests/vmTests.py
File tests/vmTests.py:

Line 53: 
Line 54:     def listAllNetworks(self, *args):
Line 55:         return []
Line 56: 
Line 57:     def lookupByUUIDString(self, id):
is this ever used?
Line 58:         pass
Line 59: 
Line 60: 
Line 61: class FakeDomain:


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

Line 87: CONSOLE_DEVICES = 'console'
Line 88: SMARTCARD_DEVICES = 'smartcard'
Line 89: TPM_DEVICES = 'tpm'
Line 90: 
Line 91: # The Metadata URI for VM tunables
the comment adds nothing beyond the constant name, so please drop it
Line 92: METADATA_VM_TUNE_URI = 'http://ovirt.org/vm/tune/1.0'
Line 93: 
Line 94: # A libvirt constant for undefined cpu quota
Line 95: NO_CPU_QUOTA = 0


Line 91: # The Metadata URI for VM tunables
Line 92: METADATA_VM_TUNE_URI = 'http://ovirt.org/vm/tune/1.0'
Line 93: 
Line 94: # A libvirt constant for undefined cpu quota
Line 95: NO_CPU_QUOTA = 0
Does anybody need these values out of the vm module? If not, mark them as private with a leading underscore.
Line 96: 
Line 97: # A libvirt constant for undefined cpu period
Line 98: NO_CPU_PERIOD = 0
Line 99: 


Line 98: NO_CPU_PERIOD = 0
Line 99: 
Line 100: # CPU tune sampling window
Line 101: # minimum supported value is 2
Line 102: CPU_TUNE_SAMPLING_WINDOW = 2
this constant can easily hide within the function that uses it.
Line 103: 
Line 104: 
Line 105: def isVdsmImage(drive):
Line 106:     """


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