Roy Golan has posted comments on this change.

Change subject: refine calculation of cpu topology
......................................................................


Patch Set 3:

(4 comments)

http://gerrit.ovirt.org/#/c/30896/3/vdsm/virt/vmxml.py
File vdsm/virt/vmxml.py:

Line 95: 
Line 96:         """
Line 97:         self.conf = conf
Line 98:         self.log = log
Line 99:         self._setTopology()
> better to have all the initialization directly in __init__, so I'd rather d
Done
Line 100:         self.arch = arch
Line 101: 
Line 102:         self.doc = xml.dom.minidom.Document()
Line 103: 


Line 127: ('
> too long line, that's the reason for automation's negative score.
Done


Line 428:         cores = int(self.conf.get('smpCoresPerSocket', '1'))
Line 429:         threads = int(self.conf.get('smpThreadsPerCore', '1'))
Line 430:         sockets = int(self.conf.get('maxNumberOfSockets', '16'))
Line 431:         maxVCpus = int(self.conf.get('maxVCpus', self._getSmp()))
Line 432:         maxVCpus = maxVCpus if cores * sockets > maxVCpus else cores 
* sockets
> don't we have something like a max(a, b) function?
min(a,b) is the one I guess because we don't want to allocate sockets above the 
configurable max.
Line 433: 
Line 434:         self._topology = {
Line 435:             'cores': cores,
Line 436:             'threads': threads,


Line 435:             'cores': cores,
Line 436:             'threads': threads,
Line 437:             'sockets': sockets,
Line 438:             'maxVCpus': maxVCpus
Line 439:         }
> (see comment at line 99)
Done
I got no problem with that. can you explain why? is that a matter of convention 
to so self_a = self._calcA()?
Line 440: 
Line 441: if sys.version_info[:2] == (2, 6):
Line 442:     # A little unrelated hack to make 
xml.dom.minidom.Document.toprettyxml()
Line 443:     # not wrap Text node with whitespace.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0d71dfe98b67440c084b2a7bf8b292f6b9c3ae19
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Roy Golan <[email protected]>
Gerrit-Reviewer: Francesco Romani <[email protected]>
Gerrit-Reviewer: Michal Skrivanek <[email protected]>
Gerrit-Reviewer: Michal Skrivanek <[email protected]>
Gerrit-Reviewer: Roy Golan <[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