Francesco Romani has uploaded a new change for review. Change subject: vm: introduce a `monitorable' attribute ......................................................................
vm: introduce a `monitorable' attribute In the change I3e61626625a2e0517d55dc61e361f3f5eb690c00 we fixed the stats_age reporting, in order to correctly report the real age of VM monitoring samples. In the same change we acknowledged a possible change in behaviour: depending on the timing of operations and on the load of the host, VMs could be reported 'Unresponsive' for a little time while starting up or shutting down. This information is tecnhically correct: the monitoring subsystem dutifully reports unknown sampling data, which is translated into outdated values, thus unresponsive VM, but it is misleading for both Engine and users. There are two root causes here: 1. creation, destruction and monitoring of a VM are all operations which are asynchronous to each other, for both performance and historical reasons - and both reasons are still relevant today. 2. the semantic of this reporting (VM unresponsive for stats too old) is poorly defined, so the right thing is to keep backward compatibility, lacking better description. To solve this issue we generalize the old VM.incomingMigrationPending attribute into a 'monitorable' attribute. A VM is monitorable if it is in a meaningful state, so after the creation process is completed, or before the shutdown process is initiated. Please note that the `monitorable' state share similarities with the VM state (VM.lastStatus attribute), but the two concepts overlap only for a small part. Lacking a precise definition, we use two independent variables to track those attributes. Change-Id: Idb12277a5f0fdaf680032af12fa7c104c3bd6ffb Backport-To: 4.0 Backport-To: 3.6 Bug-Url: https://bugzilla.redhat.com/1382578 Bug-Url: https://bugzilla.redhat.com/1382583 Signed-off-by: Francesco Romani <from...@redhat.com> --- M lib/vdsm/virt/periodic.py M lib/vdsm/virt/vmstats.py M tests/vmStatsTests.py M vdsm/virt/vm.py 4 files changed, 14 insertions(+), 7 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/90/65590/1 diff --git a/lib/vdsm/virt/periodic.py b/lib/vdsm/virt/periodic.py index 132fe93..8a89870 100644 --- a/lib/vdsm/virt/periodic.py +++ b/lib/vdsm/virt/periodic.py @@ -290,7 +290,7 @@ def required(self): # Disable everything until the migration destination VM # is fully started, to avoid false positives log spam. - return not self._vm.incomingMigrationPending() + return self._vm.monitorable @property def runnable(self): diff --git a/lib/vdsm/virt/vmstats.py b/lib/vdsm/virt/vmstats.py index dadbf26..8e271cb 100644 --- a/lib/vdsm/virt/vmstats.py +++ b/lib/vdsm/virt/vmstats.py @@ -495,7 +495,7 @@ try: yield except KeyError: - if vm_obj.incomingMigrationPending(): + if not vm_obj.monitorable: # If a VM is migration destination, # libvirt doesn't give any disk stat. pass diff --git a/tests/vmStatsTests.py b/tests/vmStatsTests.py index 9827c4e..6c57696 100644 --- a/tests/vmStatsTests.py +++ b/tests/vmStatsTests.py @@ -580,8 +580,9 @@ self.drives = drives if drives is not None else [] self.migrationPending = False - def incomingMigrationPending(self): - return self.migrationPending + @property + def monitorable(self): + return not self.migrationPending def getNicDevices(self): return self.nics diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py index dd633e6..ded832e 100644 --- a/vdsm/virt/vm.py +++ b/vdsm/virt/vm.py @@ -307,6 +307,13 @@ self._numaInfo = {} self._vmJobs = None self._clientPort = '' + self._monitorable = False + + @property + def monitorable(self): + if 'migrationDest' in self.conf or 'restoreState' in self.conf: + return False + return self._monitorable @property def start_time(self): @@ -585,9 +592,6 @@ if acquired: self.log.debug('Releasing incoming migration semaphore') migration.incomingMigrations.release() - - def incomingMigrationPending(self): - return 'migrationDest' in self.conf or 'restoreState' in self.conf def disableDriveMonitor(self): self._driveMonitorEnabled = False @@ -1669,6 +1673,7 @@ raise MissingLibvirtDomainError(vmexitreason.LIBVIRT_START_FAILED) sampling.stats_cache.add(self.id) + self._monitorable = True self._guestEventTime = self._startTime @@ -3936,6 +3941,7 @@ nic.portMirroring.remove(network) self.log.info('Release VM resources') + self._monitorable = False self.lastStatus = vmstatus.POWERING_DOWN # Terminate the VM's creation thread. self._incomingMigrationFinished.set() -- To view, visit https://gerrit.ovirt.org/65590 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Idb12277a5f0fdaf680032af12fa7c104c3bd6ffb 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
