Francesco Romani has uploaded a new change for review.
Change subject: vm: move getBalloonInfo in the stats thread
......................................................................
vm: move getBalloonInfo in the stats thread
To gather the ballon stats, we used to make a direct libvirt
call in the getStats() body. This is dangerous, because
getStats() should not contain costly calls, and should
just gather data.
This patch reimplements the balloon information getter as a
sampling function, and moves it into the VmStatsThread
to make things more robust.
As remainder to avoid to sneak costly calls in getStats,
this patch also adds a docstring in getStatsInternal.
Change-Id: I51fd1268b3718f3cb0c1bb32b13f38a58a67b8be
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
M tests/vmTests.py
M vdsm/virt/vm.py
2 files changed, 43 insertions(+), 25 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/96/27696/1
diff --git a/tests/vmTests.py b/tests/vmTests.py
index 1dd75ae..13e3f5a 100644
--- a/tests/vmTests.py
+++ b/tests/vmTests.py
@@ -912,7 +912,9 @@
# bz1073478 - main case
with FakeVM(self.VM_PARAMS, self.DEV_BALLOON) as fake:
self.assertEqual(fake._dom, None)
- res = fake.getStats()
+ mock_stats_thread = vm.VmStatsThread(fake)
+ res = {}
+ mock_stats_thread._getBalloonStats(res)
self.assertIn('balloonInfo', res)
self.assertIn('balloon_cur', res['balloonInfo'])
@@ -920,7 +922,9 @@
# bz1073478 - extra case
with FakeVM(self.VM_PARAMS, self.DEV_BALLOON) as fake:
fake._dom = FakeDomain()
- res = fake.getStats()
+ mock_stats_thread = vm.VmStatsThread(fake)
+ res = {}
+ mock_stats_thread._getBalloonStats(res)
self.assertIn('balloonInfo', res)
self.assertIn('balloon_cur', res['balloonInfo'])
@@ -994,6 +998,7 @@
vmParams = {
'displayPort': -1, 'displaySecurePort': -1,
'display': 'qxl',
'displayIp': '127.0.0.1', 'vmType': 'kvm',
'devices': {},
+ 'memSize': 1024,
# HACKs
'pauseCode': 'NOERR'}
with FakeVM(vmParams) as fake:
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index f1c36f8..13ceba4 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -203,10 +203,14 @@
self._sampleNet,
config.getint('vars', 'vm_sample_net_interval'),
config.getint('vars', 'vm_sample_net_window')))
+ self.sampleBalloon = (
+ sampling.AdvancedStatsFunction(
+ self._sampleBalloon))
self.addStatsFunction(
self.highWrite, self.updateVolumes, self.sampleCpu,
- self.sampleDisk, self.sampleDiskLatency, self.sampleNet)
+ self.sampleDisk, self.sampleDiskLatency, self.sampleNet,
+ self.sampleBalloon)
def _highWrite(self):
if not self._vm.isDisksStatsCollectionEnabled():
@@ -257,6 +261,10 @@
netSamples[nic.name] = self._vm._dom.interfaceStats(nic.name)
return netSamples
+ def _sampleBalloon(self):
+ infos = self._vm._dom.info()
+ return infos[2]
+
def _diff(self, prev, curr, val):
return prev[val] - curr[val]
@@ -281,6 +289,25 @@
self._log.debug("CPU stats not available: %s", e)
stats['cpuUser'] = 0.0
stats['cpuSys'] = 0.0
+
+ def _getBalloonStats(self, stats):
+ maxMem = int(self._vm.conf.get('memSize')) * 1024
+
+ sInfo, eInfo, sampleInterval = self.sampleBalloon.getStats()
+
+ ballonInfo = {
+ 'balloon_max': str(maxMem),
+ 'balloon_min': str(
+ int(self._vm.conf.get('memGuaranteedSize', '0')) *
1024),
+ 'balloon_cur': str(sInfo) if sInfo is not None else '0',
+ 'balloon_target': '0'}
+
+ for dev in self._vm.conf['devices']:
+ if dev['type'] == BALLOON_DEVICES and \
+ dev['specParams']['model'] != 'none':
+ ballonInfo['balloon_target'] = dev.get('target', maxMem)
+
+ stats['balloonInfo'] = ballonInfo
@classmethod
def _getNicStats(cls, name, model, mac,
@@ -402,6 +429,7 @@
self._getNetworkStats(stats)
self._getDiskStats(stats)
self._getDiskLatency(stats)
+ self._getBalloonStats(stats)
return stats
@@ -2428,7 +2456,11 @@
return stats
def _getStatsInternal(self):
- # used by API.Vm.getStats
+ """
+ WARNING: this method should only gather statistics by copying data.
+ Especially avoid costly and dangerous ditrect calls to the _dom
+ attribute. Use the VmStatsThread instead!
+ """
def _getGuestStatus():
GUEST_WAIT_TIMEOUT = 60
@@ -2477,6 +2509,8 @@
stats[var] = utils.convertToStr(decStats[var])
elif var == 'network':
stats['network'] = decStats[var]
+ elif var == 'balloonInfo':
+ stats['balloonInfo'] = decStats[var]
else:
try:
stats['disks'][var] = {}
@@ -2516,7 +2550,6 @@
memUsage = (100 - float(realMemUsage) /
int(self.conf['memSize']) * 100)
stats['memUsage'] = utils.convertToStr(int(memUsage))
- stats['balloonInfo'] = self._getBalloonInfo()
if self.isMigrating():
stats['migrationProgress'] =
self.migrateStatus()['progress']
return stats
@@ -4362,26 +4395,6 @@
self.deleteVm()
return {'status': doneCode}
-
- def _getBalloonInfo(self):
- for dev in self.conf['devices']:
- if dev['type'] == BALLOON_DEVICES and \
- dev['specParams']['model'] != 'none':
- max_mem = int(self.conf.get('memSize')) * 1024
- min_mem = int(self.conf.get('memGuaranteedSize', '0')) *
1024
- target_mem = dev.get('target', max_mem)
- try:
- cur_mem = self._dom.info()[2]
- except (AttributeError, libvirt.libvirtError):
- # _dom may be None (race on shutdown)
- self.log.exception(
- 'failed to retrieve the balloon stats')
- cur_mem = 0
- return {'balloon_max': str(max_mem),
- 'balloon_cur': str(cur_mem),
- 'balloon_min': str(min_mem),
- 'balloon_target': str(target_mem)}
- return {}
def setBalloonTarget(self, target):
--
To view, visit
http://gerrit.ovirt.org/27696
To unsubscribe, visit
http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I51fd1268b3718f3cb0c1bb32b13f38a58a67b8be
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>