Ayal Baron has posted comments on this change.
Change subject: virtEnabledInCpuAndBios: return status/proper msg
......................................................................
Patch Set 3: I would prefer that you didn't submit this
(6 inline comments)
....................................................
File vds_bootstrap/vds_bootstrap.py
Line 276: self.message = ''
Line 277: self.rc = True
Line 278:
Line 279: if self.rc:
Line 280: retCPU, msg = deployUtil.virtEnabledInCpuAndBios()
s/retCPU/virtSupport/ ?
Line 281: if retCPU:
Line 282: self.vt_svm = "OK"
Line 283: self.message = msg
Line 284: else:
....................................................
File vdsm_reg/deployUtil.py.in
Line 1299: elif v == 'AuthenticAMD':
Line 1300: return v
Line 1301: return ''
Line 1302:
Line 1303: def virtEnabledInCpuAndBios():
+1
In the least need a TODO
Line 1304: try:
Line 1305: vendor = cpuVendorID()
Line 1306: logging.debug('CPU vendor is %s', vendor)
Line 1307:
Line 1304: try:
Line 1305: vendor = cpuVendorID()
Line 1306: logging.debug('CPU vendor is %s', vendor)
Line 1307:
Line 1308: msg = "Server supports virtualization"
This message should not be configured here as Dan said.
Line 1309:
Line 1310: if vendor == 'GenuineIntel':
Line 1311: bios_en = _vmx_enabled_by_bios()
Line 1312: has_cpu = _cpu_has_vmx_support();
Line 1319:
Line 1320: logging.debug('Virtualization support in cpu: %s, in bios:
%s',
Line 1321: has_cpu, bios_en)
Line 1322:
Line 1323: if has_cpu and bios_en == False:
The semantics here are weird.
either 'if has_cpu and not bios_en'
or 'if has_cpu == True and bios_en == False'
Line 1324: msg = 'Please enable the virtualization support in BIOS!'
Line 1325:
Line 1326: if has_cpu == False:
Line 1327: msg = 'CPU model doesn\'t have virtualization
support!'
Line 1322:
Line 1323: if has_cpu and bios_en == False:
Line 1324: msg = 'Please enable the virtualization support in BIOS!'
Line 1325:
Line 1326: if has_cpu == False:
Need to reverse the order of the two 'if's so that you'd have:
if not has_cpu:
msg = ...
elif not bios_en:
msg = ...
else:
msg = "Server supports virtualization"
Line 1327: msg = 'CPU model doesn\'t have virtualization
support!'
Line 1328:
Line 1329: return bios_en and has_cpu, msg
Line 1330: except:
Line 1328:
Line 1329: return bios_en and has_cpu, msg
Line 1330: except:
Line 1331: logging.error(traceback.format_exc())
Line 1332: return False, traceback.format_exc()
why are you returning the traceback? (at most str(e))
I would just raise here
Line 1333:
Line 1334: def getRemoteFile(IP, port, fileName, timeout=None, certPath=None):
Line 1335: logging.debug("getRemoteFile start. IP = %s port = %s fileName =
\"%s\""
Line 1336: % (IP, port, fileName))
--
To view, visit
http://gerrit.ovirt.org/3549
To unsubscribe, visit
http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3baff7594ad2cfcf4844a30586a2e4a4313a1972
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf <dougsland(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server