Change in vdsm[master]: Probe cpu count from new libvirt capabilities.

danken at redhat.com danken at redhat.com
Wed Feb 6 09:42:19 UTC 2013


Dan Kenigsberg has posted comments on this change.

Change subject: Probe cpu count from new libvirt capabilities.
......................................................................


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

(3 inline comments)

My main issue with this patch is that it is based on a libvirt feature missing from our both reference platforms.

If we take the code, wouldn't it cause getCaps to explode on el6?

....................................................
File vdsm/caps.py
Line 110:     caps = minidom.parseString(capabilities)
Line 111:     host = caps.getElementsByTagName('host')[0]
Line 112:     cells = host.getElementsByTagName('cells')[0]
Line 113:     cpus = cells.getElementsByTagName('cpu').length
Line 114:     socketList = []
It's a bit nicer to start with

 sockets = set()

and thus avoid the temporary lists.
Line 115:     siblingsList = []
Line 116:     for line in cells.getElementsByTagName('cpu'):
Line 117:         socket_id = line.getAttribute('socket_id')
Line 118:         siblings = line.getAttribute('siblings')


Line 112:     cells = host.getElementsByTagName('cells')[0]
Line 113:     cpus = cells.getElementsByTagName('cpu').length
Line 114:     socketList = []
Line 115:     siblingsList = []
Line 116:     for line in cells.getElementsByTagName('cpu'):
nit: it's not a "line", but a "cpu".
Line 117:         socket_id = line.getAttribute('socket_id')
Line 118:         siblings = line.getAttribute('siblings')
Line 119:         socketList.append(socket_id)
Line 120:         siblingsList.append(siblings)


....................................................
File vdsm.spec.in
Line 118: # Subprocess and thread bug was found on python 2.7.2
Line 119: Requires: python >= 2.7.3
Line 120: Requires: qemu-kvm >= 2:0.15.0-4
Line 121: Requires: qemu-img >= 2:0.15.0-4
Line 122: # libvirt with detailed capabilities cpu data (rhbz 888503)
I do not see why we should keep the comment here.
Line 123: # needed by vdsm to probe sockets/cores/threads information.
Line 124: Requires: libvirt >= 1.0.2-1
Line 125: Requires: iscsi-initiator-utils >= 6.2.0.872-14
Line 126: Requires: device-mapper-multipath >= 0.4.9-18


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0e4a08cfcde6fb2e1ce4cf10478af72217c20ba5
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Amador Pahim <apahim at redhat.com>
Gerrit-Reviewer: Amador Pahim <apahim at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland at redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw at linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list