Change in vdsm[master]: vm: Cleaner vm stats thread initialization

[Nir Soffer]

Nir Soffer has uploaded a new change for review.

Change subject: vm: Cleaner vm stats thread initialization
......................................................................

vm: Cleaner vm stats thread initialization

The vm stats thread was initialized too late, leading to unneeded checks
and unclear try except blocks when trying to stop the thread.

This patch changes AdvancedStatsThread so it keeps a thread instance
instead of inheriting from Thread, and initialize it in Vm.__init__, so
it is always available when we access it.

Change-Id: I2917de42b76ee3dc8b27bdc23b33f3c984a7cc68
Signed-off-by: Nir Soffer 
---
M vdsm/virt/sampling.py
M vdsm/virt/vm.py
2 files changed, 11 insertions(+), 18 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/99/39299/1

diff --git a/vdsm/virt/sampling.py b/vdsm/virt/sampling.py
index 3206b97..848bb36 100644
--- a/vdsm/virt/sampling.py
+++ b/vdsm/virt/sampling.py
@@ -401,7 +401,7 @@
         return self._samples.last()

-class AdvancedStatsThread(threading.Thread):
+class AdvancedStatsThread(object):
     """
     A thread that runs the registered AdvancedStatsFunction objects
     for statistic and monitoring purpose.
@@ -412,9 +412,8 @@
         """
         Initialize an AdvancedStatsThread object
         """
-        threading.Thread.__init__(self)
         self.daemon = daemon
-
+        self._thread = None
         self._log = log
         self._stopEvent = threading.Event()
         self._contEvent = threading.Event()
@@ -426,7 +425,7 @@
         """
         Register the functions listed as arguments
         """
-        if self.isAlive():
+        if self._thread is not None:
             raise RuntimeError("AdvancedStatsThread is started")

for statsFunction in args:
@@ -437,7 +436,9 @@
         Start the execution of the thread and exit
         """
         self._log.debug("Start statistics collection")
-        threading.Thread.start(self)
+        self._thread = threading.Thread(target=self._run)
+        self._thread.daemon = self.daemon
+        self._thread.start()

def stop(self):
         """
@@ -464,7 +465,7 @@
     def getLastSampleTime(self):
         return self._statsTime

-    def run(self):
+    def _run(self):
         self._log.debug("Stats thread started")
         self._contEvent.set()

diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index 28054bf..1bc04e7 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -779,7 +779,7 @@
         self._initTimeRTC = long(self.conf.get('timeOffset', 0))
         self._guestEvent = vmstatus.POWERING_UP
         self._guestEventTime = 0
-        self._vmStats = None
+        self._vmStats = VmStatsThread(self)
         self._guestCpuRunning = False
         self._guestCpuLock = threading.Lock()
         self._startTime = time.time() - \
@@ -1269,7 +1269,7 @@
             return
         toSave = self.status()
         toSave['startTime'] = self._startTime
-        if self.lastStatus != vmstatus.DOWN and self._vmStats:
+        if self.lastStatus != vmstatus.DOWN:
             guestInfo = self.guestAgent.getGuestInfo()
             toSave['username'] = guestInfo['username']
             toSave['guestIPs'] = guestInfo['guestIPs']
@@ -1758,7 +1758,7 @@

decStats = {}
         try:
-            if self._vmStats and self._vmStats.getLastSampleTime() is not None:
+            if self._vmStats.getLastSampleTime() is not None:
                 decStats = self._vmStats.get()
                 self._setUnresponsiveIfTimeout(
                     stats,
@@ -2001,19 +2001,11 @@
         return domxml.toxml()

def startVmStats(self):
-        self._vmStats = VmStatsThread(self)
         self._vmStats.start()
         self._guestEventTime = self._startTime

def stopVmStats(self):
-        # this is less clean that it could be, but we can get here from
-        # many flows and with various locks held
-        # (_releaseLock, _shutdownLock)
-        # _vmStats may be None already, and we're good with that.
-        try:
-            self._vmStats.stop()
-        except AttributeError:
-            pass
+        self._vmStats.stop()

@staticmethod
     def _guestSockCleanup(sock):