Change in vdsm[master]: migration: don't inherit from threading.Thread

fromani at redhat.com fromani at redhat.com
Thu Dec 17 07:28:36 UTC 2015


Francesco Romani has uploaded a new change for review.

Change subject: migration: don't inherit from threading.Thread
......................................................................

migration: don't inherit from threading.Thread

To have one class which inherits from threading.Thread is
bad practice. Better to have composition than inheritance.

Change-Id: I1385121990a885584fce607ce12f40011e67973d
Signed-off-by: Francesco Romani <fromani at redhat.com>
---
M vdsm/virt/migration.py
1 file changed, 37 insertions(+), 27 deletions(-)


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

diff --git a/vdsm/virt/migration.py b/vdsm/virt/migration.py
index 4b9a2b9..78ea855 100644
--- a/vdsm/virt/migration.py
+++ b/vdsm/virt/migration.py
@@ -73,7 +73,7 @@
     STALLED = 3
 
 
-class SourceThread(threading.Thread):
+class SourceThread(object):
     """
     A thread that takes care of migration on the source vdsm.
     """
@@ -112,10 +112,19 @@
         self._last_error = None
         self._last_status = MigrationStatus.IN_PROGRESS
         self._progress = 0
-        threading.Thread.__init__(self)
         self._migrationPrepared = threading.Event()
         self._migrationCanceled = threading.Event()
         self._monitorThread = None
+        self._destServer = None
+
+        self._thread = threading.Thread(target=self._run)
+        self._thread.daemon = True
+        self._thread.start()
+
+    def is_alive(self):
+        return self._thread.is_alive()
+
+    isAlive = is_alive
 
     @property
     def hibernating(self):
@@ -310,7 +319,7 @@
                  -1, -1)                             # int1, int2
         raise e
 
-    def run(self):
+    def _run(self):
         try:
             startTime = time.time()
             self._setupVdsConnection()
@@ -465,22 +474,27 @@
         yield int(offset + base ** i)
 
 
-class DowntimeThread(threading.Thread):
+class DowntimeThread(object):
     def __init__(self, vm, downtime, steps):
-        super(DowntimeThread, self).__init__()
-
         self._vm = vm
         self._downtime = downtime
         self._steps = steps
+
         self._stop = threading.Event()
+
+        self._thread = threading.Thread(target=self._run)
+        self._thread.daemon = True
+        self._thread.start()
 
         delay_per_gib = config.getint('vars', 'migration_downtime_delay')
         memSize = int(vm.conf['memSize'])
         self._wait = (delay_per_gib * max(memSize, 2048) + 1023) / 1024
 
-        self.daemon = True
+    def stop(self):
+        self._vm.log.debug('stopping migration downtime thread')
+        self._stop.set()
 
-    def run(self):
+    def _run(self):
         self._vm.log.debug('migration downtime thread started (%i steps)',
                            self._steps)
 
@@ -490,10 +504,6 @@
             self._set_downtime(self._downtime)
 
         self._vm.log.debug('migration downtime thread exiting')
-
-    def stop(self):
-        self._vm.log.debug('stopping migration downtime thread')
-        self._stop.set()
 
     def _set_downtime_by_steps(self, max_downtime):
         for downtime in exponential_downtime(max_downtime, self._steps):
@@ -509,30 +519,34 @@
         self._vm._dom.migrateSetMaxDowntime(downtime, 0)
 
 
-class MonitorThread(threading.Thread):
+class MonitorThread(object):
+
     _MIGRATION_MONITOR_INTERVAL = config.getint(
         'vars', 'migration_monitor_interval')  # seconds
 
     def __init__(self, vm, startTime):
-        super(MonitorThread, self).__init__()
-        self._stop = threading.Event()
         self._vm = vm
         self._startTime = startTime
-        self.daemon = True
         self.progress = 0
 
+        self._stop = threading.Event()
+
+        if self.enabled:
+            self._thread = threading.Thread(target=self._run)
+            self._thread.daemon = True
+            self._thread.start()
+        else:
+            self._vm.log.info('migration monitor thread disabled'
+                              ' (monitoring interval set to 0)')
     @property
     def enabled(self):
         return MonitorThread._MIGRATION_MONITOR_INTERVAL > 0
 
-    def run(self):
-        if self.enabled:
-            self.monitor_migration()
-        else:
-            self._vm.log.info('migration monitor thread disabled'
-                              ' (monitoring interval set to 0)')
+    def stop(self):
+        self._vm.log.debug('stopping migration monitor thread')
+        self._stop.set()
 
-    def monitor_migration(self):
+    def _run(self):
         def update_progress(remaining, total):
             if remaining == 0 and total:
                 return 100
@@ -595,7 +609,3 @@
                 self._vm.log.info('Migration Progress: %s seconds elapsed,'
                                   ' %s%% of data processed' %
                                   (timeElapsed / 1000, self.progress))
-
-    def stop(self):
-        self._vm.log.debug('stopping migration monitor thread')
-        self._stop.set()


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I1385121990a885584fce607ce12f40011e67973d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>


More information about the vdsm-patches mailing list