Change in vdsm[master]: vdsm: introduce cpuinfo module

fromani at redhat.com fromani at redhat.com
Mon Oct 5 07:02:11 UTC 2015


Francesco Romani has posted comments on this change.

Change subject: vdsm: introduce cpuinfo module
......................................................................


Patch Set 7: Code-Review+1

(7 comments)

looks good. Few minor comments and questions inside

https://gerrit.ovirt.org/#/c/46912/7/debian/vdsm-tests.install
File debian/vdsm-tests.install:

Line 8: usr/share/vdsm/tests/caps_libvirt_intel_i73770.out
Line 9: usr/share/vdsm/tests/caps_libvirt_intel_i73770_nosnap.out
Line 10: usr/share/vsdm/tests/caps_numactl_4_nodes.out
Line 11: usr/share/vdsm/tests/cpu_map.xml
Line 12: usr/share/vdsm/tests/cpu/*.out
I still believe cpuinfo/ is more apt, but no bug deal.
Line 13: usr/share/vdsm/tests/devices/parsing*.py
Line 14: usr/share/vdsm/tests/devices/data/*.xml
Line 15: usr/share/vdsm/tests/functional/*.policy
Line 16: usr/share/vdsm/tests/functional/*.py


https://gerrit.ovirt.org/#/c/46912/7/lib/vdsm/cpuinfo.py
File lib/vdsm/cpuinfo.py:

Line 67:     if targetarch == Architecture.X86_64:
Line 68:         return 'x86'
Line 69: 
Line 70:     if targetarch in Architecture.POWER:
Line 71:         return 'ppc64'
else?
Line 72: 
Line 73: 
Line 74: def parse(source='/proc/cpuinfo'):
Line 75:     '''


Line 89:             except ValueError:
Line 90:                 continue
Line 91:             if key == 'processor':
Line 92:                 cpu = int(value)
Line 93:                 _CPUS[cpu] = {}
maybe make _CPUS a defaultdict(dict) ?
Line 94:                 continue
Line 95:             else:
Line 96:                 _CPUS[cpu][key] = value
Line 97: 


Line 188:     if not _CPUS:
        :         parse()
for some reasons I still think a eager initialization (cpuinfo.parse() somewhere at startup) would be nicer.


https://gerrit.ovirt.org/#/c/46912/7/lib/vdsm/utils.py
File lib/vdsm/utils.py:

Line 808:         if os.path.exists(constants.P_VDSM_NODE_ID):
Line 809:             with open(constants.P_VDSM_NODE_ID) as f:
Line 810:                 __hostUUID = f.readline().replace("\n", "")
Line 811:         else:
Line 812:             if cpuinfo.arch() in (cpuinfo.Architecture.X86_64):
parenthesis here seems unneeded
Line 813:                 ret, out, err = execCmd([constants.EXT_DMIDECODE,
Line 814:                                          "-s",
Line 815:                                          "system-uuid"],
Line 816:                                         raw=True,


https://gerrit.ovirt.org/#/c/46912/7/tests/Makefile.am
File tests/Makefile.am:

Line 20: 
Line 21: include $(top_srcdir)/build-aux/Makefile.subs
Line 22: 
Line 23: SUBDIRS = \
Line 24: 	  cpu\
missing space before "\"
Line 25: 	  functional \
Line 26: 	  devices \
Line 27: 	  integration \
Line 28: 	  $(NULL)


https://gerrit.ovirt.org/#/c/46912/7/tests/nettestlib.py
File tests/nettestlib.py:

Line 103:         _TUNSETIFF = 0x400454ca
Line 104:     elif cpuinfo.arch() == 'ppc64':
Line 105:         _TUNSETIFF = 0x800454ca
Line 106:     else:
Line 107:         raise cpuinfo.UnsupportedArchitecture
not sure. We may probably want to skip this test, not to make it fail.
Line 108: 
Line 109:     _deviceListener = None
Line 110: 
Line 111:     def addDevice(self):


-- 
To view, visit https://gerrit.ovirt.org/46912
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaa702b05f3825ebdcfed16d86d39a8c38fcf224c
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list