Francesco Romani has uploaded a new change for review.
Change subject: vm: refactor the monitorResponse handling
......................................................................
vm: refactor the monitorResponse handling
Move everything in a single method, to make the flow
easier to understand and change.
No expected changes in behaviour.
Change-Id: Ic48d88dfd8cb1ad086be4b0e182fab02462f52c9
Signed-off-by: Francesco Romani <from...@redhat.com>
---
M tests/vmTests.py
M vdsm/virt/vm.py
2 files changed, 43 insertions(+), 38 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/39/65539/1
diff --git a/tests/vmTests.py b/tests/vmTests.py
index 893bc74..993fa4e 100644
--- a/tests/vmTests.py
+++ b/tests/vmTests.py
@@ -40,7 +40,7 @@
from vdsm.virt import vmstatus
from virt import vm
-from virt.vm import HotunplugTimeout
+from virt.vm import HotunplugTimeout, VMMonitorState
from virt import vmdevices
from virt.vmdevices import hwclass
from virt.vmtune import io_tune_merge, io_tune_dom_to_values, io_tune_to_dom
@@ -1421,9 +1421,9 @@
def testMonitorTimeoutResponsive(self):
with fake.VM(_VM_PARAMS) as testvm:
self.assertFalse(testvm.isMigrating())
- stats = {'monitorResponse': '0'}
- testvm._setUnresponsiveIfTimeout(stats, 1) # any value < timeout
- self.assertEqual(stats['monitorResponse'], '0')
+ self.assertEquals(
+ testvm._getVmMonitorResponse(1), # any value < timeout
+ VMMonitorState.READY)
@MonkeyPatch(vm, 'config',
make_config([('vars', 'vm_command_timeout', '1')]))
@@ -1434,11 +1434,11 @@
])
def testMonitorTimeoutUnresponsive(self, runCpu):
with fake.VM(_VM_PARAMS, runCpu=runCpu, status=vmstatus.UP) as testvm:
- self.assertEqual(testvm._monitorResponse, 0)
+ self.assertEqual(testvm._monitorResponse, VMMonitorState.READY)
self.assertFalse(testvm.isMigrating())
- stats = {'monitorResponse': '0'}
- testvm._setUnresponsiveIfTimeout(stats, 10) # any value > timeout
- self.assertEqual(stats['monitorResponse'], '-1')
+ self.assertEquals(
+ testvm._getVmMonitorResponse(10), # any value > timeout
+ VMMonitorState.UNRESPONSIVE)
@MonkeyPatch(vm, 'config',
make_config([('vars', 'vm_command_timeout', '10')]))
@@ -1449,11 +1449,12 @@
])
def testMonitorTimeoutOnAlreadyUnresponsive(self, runCpu):
with fake.VM(_VM_PARAMS, runCpu=runCpu, status=vmstatus.UP) as testvm:
- self._monitorResponse = -1
+ # fake existing timeout
+ testvm._monitorResponse = VMMonitorState.UNRESPONSIVE
self.assertFalse(testvm.isMigrating())
- stats = {'monitorResponse': '-1'}
- testvm._setUnresponsiveIfTimeout(stats, 1) # any value < timeout
- self.assertEqual(stats['monitorResponse'], '-1')
+ self.assertEquals(
+ testvm._getVmMonitorResponse(1), # any value < timeout
+ VMMonitorState.UNRESPONSIVE)
@MonkeyPatch(vm, 'config',
make_config([('vars', 'vm_command_timeout', '1')]))
@@ -1471,10 +1472,10 @@
])
def testMonitorUncheckedStatus(self, status):
with fake.VM(_VM_PARAMS, runCpu=True, status=status) as testvm:
- self.assertEqual(testvm._monitorResponse, 0)
- stats = {'monitorResponse': '0'}
- testvm._setUnresponsiveIfTimeout(stats, 10) # any value > timeout
- self.assertEqual(stats['monitorResponse'], '0')
+ self.assertEqual(testvm._monitorResponse, VMMonitorState.READY)
+ self.assertEquals(
+ testvm._getVmMonitorResponse(10), # any value > timeout
+ VMMonitorState.READY)
class TestLibVirtCallbacks(TestCaseBase):
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index cbcc037..3550a52 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -122,6 +122,11 @@
REBOOT = 'REBOOT'
+class VMMonitorState:
+ READY = '0'
+ UNRESPONSIVE = '-1'
+
+
# These strings are representing libvirt virDomainEventType values
# http://libvirt.org/html/libvirt-libvirt-domain.html#virDomainEventType
_EVENT_STRINGS = (
@@ -241,7 +246,7 @@
self.log = SimpleLogAdapter(self.log, {"vmId": self.conf['vmId']})
self._destroy_requested = threading.Event()
self._recovery_file = recovery.File(self.conf['vmId'])
- self._monitorResponse = 0
+ self._monitorResponse = VMMonitorState.READY
self.memCommitted = 0
self._consoleDisconnectAction = ConsoleDisconnectAction.LOCK_SCREEN
self._confLock = threading.Lock()
@@ -1249,7 +1254,6 @@
"""
stats = {
'elapsedTime': str(int(time.time() - self._startTime)),
- 'monitorResponse': str(self._monitorResponse),
'timeOffset': self.conf.get('timeOffset', '0'),
'clientIp': self.conf.get('clientIp', ''),
}
@@ -1266,11 +1270,14 @@
vm_sample.first_value,
vm_sample.last_value,
vm_sample.interval)
- self._setUnresponsiveIfTimeout(stats, vm_sample.stats_age)
except Exception:
self.log.exception("Error fetching vm stats")
+ stats_age = 0
else:
stats.update(vmstats.translate(decStats))
+ stats_age = vm_sample.stats_age
+
+ stats['monitorResponse'] = self._getVmMonitorResponse(stats_age)
stats.update(self._getGraphicsStats())
stats['hash'] = str(hash((self._domain.devices_hash,
@@ -1286,6 +1293,20 @@
stats.update(self._getVmTuneStats())
return stats
+
+ def _getVmMonitorResponse(self, stats_age):
+ if self._monitorResponse == VMMonitorState.UNRESPONSIVE:
+ return VMMonitorState.UNRESPONSIVE
+
+ if (self.lastStatus in (vmstatus.UP, vmstatus.PAUSED)
+ and not self.isMigrating()):
+ if stats_age > config.getint('vars', 'vm_command_timeout'):
+ self.log.warning('monitor become unresponsive'
+ ' (command timeout, age=%s)',
+ stats_age)
+ return VMMonitorState.UNRESPONSIVE
+
+ return VMMonitorState.READY
def _getVmTuneStats(self):
stats = {}
@@ -2915,9 +2936,9 @@
def _timeoutExperienced(self, timeout):
if timeout:
- self._monitorResponse = -1
+ self._monitorResponse = VMMonitorState.UNRESPONSIVE
else:
- self._monitorResponse = 0
+ self._monitorResponse = VMMonitorState.READY
def _completeIncomingMigration(self):
if 'restoreState' in self.conf:
@@ -4669,23 +4690,6 @@
self.log.info('CPU %s: %s',
'running' if self._guestCpuRunning else 'stopped',
reason)
-
- def _setUnresponsiveIfTimeout(self, stats, stats_age):
- # we care about timeouts only on the stady state
- if self.lastStatus not in (vmstatus.UP, vmstatus.PAUSED):
- return
- if self.isMigrating():
- return
- # we don't care about decimals here
- if stats_age < config.getint('vars', 'vm_command_timeout'):
- return
- if stats['monitorResponse'] == '-1':
- return
-
- self.log.warning('monitor become unresponsive'
- ' (command timeout, age=%s)',
- stats_age)
- stats['monitorResponse'] = '-1'
def updateNumaInfo(self):
self._numaInfo = numa.getVmNumaNodeRuntimeInfo(self)
--
To view, visit https://gerrit.ovirt.org/65539
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic48d88dfd8cb1ad086be4b0e182fab02462f52c9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <from...@redhat.com>
_______________________________________________
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
