Change in vdsm[master]: vdsm: Hardware information about POWER hosts

danken at redhat.com danken at redhat.com
Fri Nov 22 21:45:12 UTC 2013


Dan Kenigsberg has posted comments on this change.

Change subject: vdsm: Hardware information about POWER hosts
......................................................................


Patch Set 3: Code-Review-1

(3 comments)

Thanks!

....................................................
File lib/vdsm/ppc64HardwareInfo.py
Line 15: #
Line 16: # Refer to the README and COPYING files for full details of the license
Line 17: #
Line 18: 
Line 19: from vdsm import utils
exposing another module in lib/vdsm is a liability we'd rather avoid. I do not see a reason not to keep this module under /usr/share/vdsm/, just like caps.py.
Line 20: 
Line 21: import os
Line 22: 
Line 23: 


....................................................
File vdsm.spec.in
Line 1039: %{python_sitearch}/%{vdsm_name}/libvirtconnection.py*
Line 1040: %{python_sitearch}/%{vdsm_name}/netinfo.py*
Line 1041: 
Line 1042: %ifarch ppc64
Line 1043: %{python_sitearch}/%{vdsm_name}/ppc64HardwareInfo.py*
please add this new file to the debian package. At least as a

 #TODO: add ppc64HardwareInfo.py for ppc64
Line 1044: %endif
Line 1045: 
Line 1046: %{python_sitearch}/%{vdsm_name}/qemuImg.py*
Line 1047: %{python_sitearch}/%{vdsm_name}/SecureXMLRPCServer.py*


....................................................
File vdsm/supervdsmServer
Line 117:     def getHardwareInfo(self, *args, **kwargs):
Line 118:         if platform.machine() in ('x86_64', 'i686'):
Line 119:             from dmidecodeUtil import getHardwareInfoStructure
Line 120:             return getHardwareInfoStructure()
Line 121:         elif platform.machine() in ('ppc64'):
this condition would evaluate to True even for platform.machine() == 'p'.
I do not suppose this is going to happen, but this is a bad practice. Please have add a coma

 elif platform.machine() in ('ppc64', ):

or move to

 elif platform.machine() == 'ppc64'
Line 122:             from vdsm.ppc64HardwareInfo import getHardwareInfoStructure
Line 123:             return getHardwareInfoStructure()
Line 124:         else:
Line 125:             #  not implemented over other architecture


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ice513b9386273a44146852944741578023c1e17a
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vitor de Lima <vitor.lima at eldorado.org.br>
Gerrit-Reviewer: Better Saggi <bettersaggi at gmail.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Gustavo Frederico Temple Pedrosa <gustavo.pedrosa at eldorado.org.br>
Gerrit-Reviewer: Leonardo Bianconi <leonardo.bianconi at eldorado.org.br>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Vitor de Lima <vitor.lima at eldorado.org.br>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list