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@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):