Antoni Segura Puimedon has posted comments on this change.
Change subject: add and use hypervisor autodetection in bootstrap.
......................................................................
Patch Set 1: (5 inline comments)
Some minor comments
....................................................
File vdsm_reg/deployUtil.py.in
Line 1325: return v
Line 1326: return ''
Line 1327:
Line 1328: # we cannot (yet) use _cpuid because of the different
Line 1329: # unpack format.
If this comment belongs to the method below, it should probably be put inside
the definition.
Line 1330: def cpuHypervisorID():
Line 1331: HYPERVISOR_CPUID_LEAF = 0x40000000
Line 1332: f = file('/dev/cpu/0/cpuid')
Line 1333: f.seek(HYPERVISOR_CPUID_LEAF)
Line 1328: # we cannot (yet) use _cpuid because of the different
Line 1329: # unpack format.
Line 1330: def cpuHypervisorID():
Line 1331: HYPERVISOR_CPUID_LEAF = 0x40000000
Line 1332: f = file('/dev/cpu/0/cpuid')
'file' is on the way out in Python and generally recommended to be substituted
by 'open'.
I would strongly recommend doing the file handling using a 'with' statement:
with open('/dev/cpu/0/cpuid') as f:
This way even in case of exception the file will get closed after the block.
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:
Line 1334: c = struct.unpack('I12s', f.read(16))
Line 1335: return c[1].strip('\x00')
Line 1336:
Line 1337: def cpuModelName():
Line 1338: for line in file('/proc/cpuinfo').readlines():
file -> open.
Even though the GC will close the file, doing it in the block in more
deterministic (Consider using the with construction).
Line 1339: if ':' in line:
Line 1340: k, v = line.split(':', 1)
Line 1341: k = k.strip()
Line 1342: v = v.strip()
Line 1338: for line in file('/proc/cpuinfo').readlines():
Line 1339: if ':' in line:
Line 1340: k, v = line.split(':', 1)
Line 1341: k = k.strip()
Line 1342: v = v.strip()
Since we are only interested in this string when k matches 'model name', maybe
we can defer the strip of v until the return part.
Line 1343: if k == 'model name':
Line 1344: return v
Line 1345: return ''
Line 1346:
Line 1339: if ':' in line:
Line 1340: k, v = line.split(':', 1)
Line 1341: k = k.strip()
Line 1342: v = v.strip()
Line 1343: if k == 'model name':
This k testing does not need to happen when ':' is not in the line.
Line 1344: return v
Line 1345: return ''
Line 1346:
Line 1347: def findHypervisor():
--
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: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <[email protected]>
Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]>
Gerrit-Reviewer: Douglas Schilling Landgraf <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
