Martin Polednik has uploaded a new change for review.
Change subject: virt: un-memoize getNumaTopology ......................................................................
virt: un-memoize getNumaTopology
The numa topology reflects which CPUs belong to which node. There is an issue with memoization in this case:
If CPU is onlined or offlined while VDSM is running, the changes will not be reflected. Anything attempting to get the CPUs from this call (such as HostStatsThread) will not be updated and will try to sample invalid CPUs.
Change-Id: I3e28688fabf8fc18252b3a35dfdd9a5a4071685b Bug-Url: https://bugzilla.redhat.com/?????? Signed-off-by: Martin Polednik mpolednik@redhat.com --- M vdsm/caps.py 1 file changed, 0 insertions(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/06/48006/1
diff --git a/vdsm/caps.py b/vdsm/caps.py index 862f8cb..41b9b76 100644 --- a/vdsm/caps.py +++ b/vdsm/caps.py @@ -302,7 +302,6 @@ return True
-@utils.memoized def getNumaTopology(capabilities=None): if capabilities is None: capabilities = _getCapsXMLStr()
automation@ovirt.org has posted comments on this change.
Change subject: virt: un-memoize getNumaTopology ......................................................................
Patch Set 1:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Francesco Romani has posted comments on this change.
Change subject: virt: un-memoize getNumaTopology ......................................................................
Patch Set 1: Code-Review-1
(1 comment)
I reviewed the users of this function. It seems we don't have a choice, we must drop the memoziation in every place.
We can do some smart tricks to optimize the access to numa info, but let's save them for future patches.
https://gerrit.ovirt.org/#/c/48006/1/vdsm/caps.py File vdsm/caps.py:
Line 303: Line 304: Line 305: def getNumaTopology(capabilities=None): Line 306: if capabilities is None: Line 307: capabilities = _getCapsXMLStr() this is memoized as well, so we didn't gain that much. Let's have this function call getFreshCapsXml if capabilities is None. Line 308: caps = ET.fromstring(capabilities) Line 309: host = caps.find('host') Line 310: cells = host.find('.//cells') Line 311: cellsInfo = {}
Francesco Romani has posted comments on this change.
Change subject: virt: un-memoize getNumaTopology ......................................................................
Patch Set 1:
(1 comment)
please see inline comments for reasons for -1
https://gerrit.ovirt.org/#/c/48006/1//COMMIT_MSG Commit Message:
Line 14: (such as HostStatsThread) will not be updated and will try to sample Line 15: invalid CPUs. Line 16: Line 17: Change-Id: I3e28688fabf8fc18252b3a35dfdd9a5a4071685b Line 18: Bug-Url: https://bugzilla.redhat.com/?????? we don't need BZs for changes in master, only for stable branches. Of course, having them also from master won't hurt :)
vdsm-patches@lists.fedorahosted.org
-
automation＠ovirt.org -
fromani＠redhat.com -
Martin Polednik