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