Change in vdsm[master]: monitor: Separate setup from monitor loop

nsoffer at redhat.com nsoffer at redhat.com
Thu May 12 12:12:45 UTC 2016


Nir Soffer has uploaded a new change for review.

Change subject: monitor: Separate setup from monitor loop
......................................................................

monitor: Separate setup from monitor loop

We have two implicit states in the monitor:

- setup         produce the domain and get iso domain prefix. If an
                exception is raised doing this, we set the monitor
                status error and try again in the next monitoring loop.

- monitor       perform various checks on the domain and update the
                status. In this state we also check and acquire the
                domain host id if needed. We also refresh the domain
                object if needed.

This patch refactor the code so these states are explicit, and the code
we run in each state is smaller and easier to grasp. This also make it
easier to integrate with new path checking infrastructure.

Change-Id: Iaa621325ddf121365c647829cc213940eae6cfa3
Signed-off-by: Nir Soffer <nsoffer at redhat.com>
---
M vdsm/storage/monitor.py
1 file changed, 67 insertions(+), 42 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/72/57372/1

diff --git a/vdsm/storage/monitor.py b/vdsm/storage/monitor.py
index f2f026d..d214e3c 100644
--- a/vdsm/storage/monitor.py
+++ b/vdsm/storage/monitor.py
@@ -237,23 +237,78 @@
     def _run(self):
         log.debug("Domain monitor for %s started", self.sdUUID)
         try:
+            self._setupLoop()
             self._monitorLoop()
+        except utils.Canceled:
+            log.debug("Domain monitor for %s canceled", self.sdUUID)
         finally:
             log.debug("Domain monitor for %s stopped (shutdown=%s)",
                       self.sdUUID, self.wasShutdown)
             if self._shouldReleaseHostId():
                 self._releaseHostId()
 
+    # Setting up
+
+    def _setupLoop(self):
+        """
+        Set up the monitor, retrying on failures. Returns when the monitor is
+        ready.
+        """
+        while True:
+            try:
+                self._setupMonitor()
+                return
+            except Exception as e:
+                log.exception("Setting up monitor for %s failed", self.sdUUID)
+                status = Status()
+                status.error = e
+                self._updateStatus(status)
+                if self.stopEvent.wait(self.interval):
+                    raise utils.Canceled
+
+    def _setupMonitor(self):
+        # Producing the domain is deferred because it might take some time and
+        # we don't want to slow down the thread start (and anything else that
+        # relies on that as for example updateMonitoringThreads). It also might
+        # fail and we want keep trying until we succeed or the domain is
+        # deactivated.
+        if self.domain is None:
+            self._produceDomain()
+
+        # The isIsoDomain assignment is deferred because the isoPrefix
+        # discovery might fail (if the domain suddenly disappears) and we
+        # could risk to never try to set it again.
+        if self.isIsoDomain is None:
+            self._setIsoDomainInfo()
+
+    @utils.cancelpoint
+    def _produceDomain(self):
+        log.debug("Producing domain %s", self.sdUUID)
+        domain = sdCache.produce(self.sdUUID)
+        self.monitoringPath = domain.getMonitoringPath()
+        self.domain = domain
+
+    @utils.cancelpoint
+    def _setIsoDomainInfo(self):
+        isIsoDomain = self.domain.isISO()
+        if isIsoDomain:
+            log.debug("Domain %s is an ISO domain", self.sdUUID)
+            self.isoPrefix = self.domain.getIsoDomainImagesDir()
+        self.isIsoDomain = isIsoDomain
+
+    # Monitoring
+
     def _monitorLoop(self):
-        while not self.stopEvent.is_set():
+        """
+        Monitor the domain peroidically until the monitor is stopped.
+        """
+        while True:
             try:
                 self._monitorDomain()
-            except utils.Canceled:
-                log.debug("Domain monitor for %s canceled", self.sdUUID)
-                return
-            except:
+            except Exception:
                 log.exception("Domain monitor for %s failed", self.sdUUID)
-            self.stopEvent.wait(self.interval)
+            if self.stopEvent.wait(self.interval):
+                raise utils.Canceled
 
     def _monitorDomain(self):
         status = Status()
@@ -263,20 +318,6 @@
             self._refreshDomain()
 
         try:
-            # We should produce the domain inside the monitoring loop because
-            # it might take some time and we don't want to slow down the thread
-            # start (and anything else that relies on that as for example
-            # updateMonitoringThreads). It also might fail and we want keep
-            # trying until we succeed or the domain is deactivated.
-            if self.domain is None:
-                self._produceDomain()
-
-            # The isIsoDomain assignment is delayed because the isoPrefix
-            # discovery might fail (if the domain suddenly disappears) and we
-            # could risk to never try to set it again.
-            if self.isIsoDomain is None:
-                self._setIsoDomainInfo()
-
             self._performDomainSelftest()
             self._checkReadDelay(status)
             self._collectStatistics(status)
@@ -285,16 +326,17 @@
             status.error = e
 
         status.checkTime = time.time()
-
-        if self._statusDidChange(status):
-            self._notifyStatusChanges(status)
+        self._updateStatus(status)
 
         if self._shouldAcquireHostId(status):
             self._acquireHostId()
 
-        self.status = FrozenStatus(status)
+    # Handling status changes
 
-    # Notifiying status changes
+    def _updateStatus(self, status):
+        if self._statusDidChange(status):
+            self._notifyStatusChanges(status)
+        self.status = FrozenStatus(status)
 
     def _statusDidChange(self, status):
         return (not self.status.actual or
@@ -321,23 +363,6 @@
         log.debug("Refreshing domain %s", self.sdUUID)
         sdCache.manuallyRemoveDomain(self.sdUUID)
         self.lastRefresh = time.time()
-
-    # Deferred initialization
-
-    @utils.cancelpoint
-    def _produceDomain(self):
-        log.debug("Producing domain %s", self.sdUUID)
-        domain = sdCache.produce(self.sdUUID)
-        self.monitoringPath = domain.getMonitoringPath()
-        self.domain = domain
-
-    @utils.cancelpoint
-    def _setIsoDomainInfo(self):
-        isIsoDomain = self.domain.isISO()
-        if isIsoDomain:
-            log.debug("Domain %s is an ISO domain", self.sdUUID)
-            self.isoPrefix = self.domain.getIsoDomainImagesDir()
-        self.isIsoDomain = isIsoDomain
 
     # Collecting monitoring info
 


-- 
To view, visit https://gerrit.ovirt.org/57372
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iaa621325ddf121365c647829cc213940eae6cfa3
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer at redhat.com>


More information about the vdsm-patches mailing list