Amador Pahim has posted comments on this change. Change subject: caps: Do not memoize CPU topology ......................................................................
Patch Set 3: Code-Review-1 (5 comments) I don't like the idea of getting rid from libvirt. We changed to libvirt some time ago to avoid code duplication and we worked with libvirt guys to fix libvirt for all corner cases we found. If you insist, please make sure all cases are covered. http://gerrit.ovirt.org/#/c/34189/3//COMMIT_MSG Commit Message: Line 3: AuthorDate: 2014-10-29 17:57:05 -0200 Line 4: Commit: Vitor de Lima <vdel...@redhat.com> Line 5: CommitDate: 2014-10-29 17:57:05 -0200 Line 6: Line 7: caps: Do not memoize CPU topology This title is the minor part of this change. Line 8: Line 9: This patch prevents the CPU topology from being memoized and implements Line 10: a fast way to retrieve this information without looking at the libvirt Line 11: capabilities XML. Line 6: Line 7: caps: Do not memoize CPU topology Line 8: Line 9: This patch prevents the CPU topology from being memoized and implements Line 10: a fast way to retrieve this information without looking at the libvirt Could you provide details on how fast is this way when compared to using libvirt? Line 11: capabilities XML. Line 12: Line 13: This is needed in order to support host CPU hotplugging, to avoid using Line 14: disabled logical CPUs and to rescan for topology changes in ppc64. http://gerrit.ovirt.org/#/c/34189/3/tests/capsTests.py File tests/capsTests.py: Line 84: @MonkeyPatch(caps, '_SYSFS_CPU_INFO_PATH', _getFakeSysfs()) Line 85: def testCpuTopology(self): Line 86: t = caps.CpuTopology() Line 87: self.assertEqual(t.threads(), 5) Line 88: self.assertEqual(t.cores(), 3) Please keep the previous tests, specially for AMD 62XX with more than one socket. Line 89: self.assertEqual(t.sockets(), 1) Line 90: Line 91: def testEmulatedMachines(self): Line 92: capsData = self._readCaps("caps_libvirt_amd_6274.out") http://gerrit.ovirt.org/#/c/34189/3/vdsm/caps.py File vdsm/caps.py: Line 240: return onlineCpus Line 241: Line 242: Line 243: def _getCpuTopology(): Line 244: sockets = set() There are cases where two distinct physical sockets have the same physical_package_id. See bz#833425. Line 245: cores = set() Line 246: onlineCpus = _getOnlineCpus() Line 247: Line 248: for cpu in onlineCpus: Line 241: Line 242: Line 243: def _getCpuTopology(): Line 244: sockets = set() Line 245: cores = set() There are cases where two distinct physical cores have the same core_id. See bz#833425. Line 246: onlineCpus = _getOnlineCpus() Line 247: Line 248: for cpu in onlineCpus: Line 249: infoPath = os.path.join(_SYSFS_CPU_INFO_PATH, 'cpu%s/topology' % cpu) -- To view, visit http://gerrit.ovirt.org/34189 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia0e87b50d4fe882240c93a0a1600e3a0cb98d443 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vitor de Lima <vdel...@redhat.com> Gerrit-Reviewer: Amador Pahim <apa...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Vitor de Lima <vdel...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
