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