Dan Kenigsberg has posted comments on this change.

Change subject: add and use hypervisor autodetection in bootstrap.
......................................................................


Patch Set 3: I would prefer that you didn't submit this

(2 inline comments)

....................................................
File vdsm_reg/deployUtil.py.in
Line 1332:     with open('/dev/cpu/0/cpuid') as f:
Line 1333:         f.seek(HYPERVISOR_CPUID_LEAF)
Line 1334:         c = struct.unpack('I12s', f.read(16))
Line 1335:         return c[1].strip('\x00')
Line 1336:     return ''
what is the point of this second "return"?
Line 1337: 
Line 1338: def cpuModelName():
Line 1339:     with open('/proc/cpuinfo') as f:
Line 1340:         for line in f:


Line 1359:             name = 'kvm'
Line 1360:         elif 'QEMU' in cpuModelName():
Line 1361:             name = 'qemu'
Line 1362:         logging.debug('detected hypervisor: %s', name)
Line 1363:     except:
catch-all "except:" is prevalent in this module, but it is evil.

is there ANY exception we would like to survive here?

I can think only of a missing or unreadable /dev/cpu/0/cpuid, which should be 
considered fatal imo.
Line 1364:         logging.error(traceback.format_exc())
Line 1365:     return name
Line 1366: 
Line 1367: def virtEnabledInCpuAndBios():


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I79f4ab08b838bd75af5d4c26f98923fca0d65d8e
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <[email protected]>
Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Douglas Schilling Landgraf <[email protected]>
Gerrit-Reviewer: Francesco Romani <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to