Change in vdsm[master]: infra: Use new concurrent.thread() utility

[Nir Soffer]

Nir Soffer has uploaded a new change for review.

Change subject: infra: Use new concurrent.thread() utility
......................................................................

infra: Use new concurrent.thread() utility

This patch updates various infra stuff to use the new utility.

Behavior changes:

- clientIF recover thread is daemonic.
- clientIF reactor thread is protected from silent failures.
- supervdsmServer server thread is protected from silent failures.

This patch does not update:

- lib/yajsonrpc - it is not clear if we want to make it depend on vdsm
  library
- debug plugin - being removed in https://gerrit.ovirt.org/44724

Change-Id: Ib7f76c4e7c6d155e97afce49e5acb55af1692cc3
Relates-To: https://bugzilla.redhat.com/1141422
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com&gt;
---
M vdsm/clientIF.py
M vdsm/rpc/bindingxmlrpc.py
M vdsm/supervdsmServer
3 files changed, 9 insertions(+), 13 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/54/45554/1

diff --git a/vdsm/clientIF.py b/vdsm/clientIF.py
index 7fa2d0d..84aaf8f 100644
--- a/vdsm/clientIF.py
+++ b/vdsm/clientIF.py
@@ -39,6 +39,7 @@
 import libvirt
 from vdsm import sslutils
 from vdsm import libvirtconnection
+from vdsm import concurrent
 from vdsm import constants
 from vdsm import utils
 from vdsm import supervdsm
@@ -108,8 +109,7 @@
             self._netConfigDirty = False
             self._prepareMOM()
             secret.clear()
-            threading.Thread(target=self._recoverThread,
-                             name='clientIFinit').start()
+            concurrent.thread(self._recoverThread, name='clientIFinit').start()
             self.channelListener.settimeout(
                 config.getint('vars', 'guest_agent_timeout'))
             self.channelListener.start()
@@ -290,9 +290,8 @@
     def start(self):
         for binding in self.bindings.values():
             binding.start()
-        self.thread = threading.Thread(target=self._reactor.process_requests,
-                                       name='Reactor thread')
-        self.thread.setDaemon(True)
+        self.thread = concurrent.thread(self._reactor.process_requests,
+                                        name='Reactor thread')
         self.thread.start()
 
     def _getUUIDSpecPath(self, uuid):
@@ -447,7 +446,6 @@
         else:
             raise JsonRpcBindingsError()
 
-    @utils.traceback()
     def _recoverThread(self):
         # Trying to run recover process until it works. During that time vdsm
         # stays in recovery mode (_recover=True), means all api requests
diff --git a/vdsm/rpc/bindingxmlrpc.py b/vdsm/rpc/bindingxmlrpc.py
index 7f1da4f..ce1b105 100644
--- a/vdsm/rpc/bindingxmlrpc.py
+++ b/vdsm/rpc/bindingxmlrpc.py
@@ -30,6 +30,7 @@
 from vdsm.password import (ProtectedPassword,
                            protect_passwords,
                            unprotect_passwords)
+from vdsm import concurrent
 from vdsm import utils
 from vdsm import xmlrpc
 from vdsm.define import doneCode, errCode
@@ -57,7 +58,6 @@
         """
         Register xml-rpc functions and serve clients until stopped
         """
-        @utils.traceback(on=self.log.name)
         def threaded_start():
             self.log.info("XMLRPC server running")
             self._registerFunctions()
@@ -73,9 +73,8 @@
                                        exc_info=True)
             self.log.info("XMLRPC server stopped")
 
-        self._thread = threading.Thread(target=threaded_start,
-                                        name='BindingXMLRPC')
-        self._thread.daemon = True
+        self._thread = concurrent.thread(threaded_start, name='BindingXMLRPC',
+                                         logger=self.log.name)
         self._thread.start()
 
     def add_socket(self, connected_socket, socket_address):
diff --git a/vdsm/supervdsmServer b/vdsm/supervdsmServer
index 3c752f8..7f7b2b3 100755
--- a/vdsm/supervdsmServer
+++ b/vdsm/supervdsmServer
@@ -24,13 +24,13 @@
 import stat
 import errno
 from functools import wraps
-import threading
 import re
 import getopt
 import resource
 import signal
 import logging
 import logging.config
+from vdsm import concurrent
 from vdsm.infra import sigutils
 from vdsm.infra import zombiereaper
 
@@ -504,8 +504,7 @@
             manager.register('instance', callable=_SuperVdsm)
 
             server = manager.get_server()
-            servThread = threading.Thread(target=server.serve_forever)
-            servThread.setDaemon(True)
+            servThread = concurrent.thread(server.serve_forever)
             servThread.start()
 
             chown(address, getpwnam(VDSM_USER).pw_uid, METADATA_GROUP)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib7f76c4e7c6d155e97afce49e5acb55af1692cc3
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com&gt;

automation(a)ovirt.org has posted comments on this change.

Change subject: infra: Use new concurrent.thread() utility
......................................................................


Patch Set 1:

* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5',
'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib7f76c4e7c6d155e97afce49e5acb55af1692cc3
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

[Piotr Kliczewski]

Piotr Kliczewski has posted comments on this change.

Change subject: infra: Use new concurrent.thread() utility
......................................................................


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib7f76c4e7c6d155e97afce49e5acb55af1692cc3
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

automation(a)ovirt.org has posted comments on this change.

Change subject: infra: Use new concurrent.thread() utility
......................................................................


Patch Set 2:

* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5',
'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib7f76c4e7c6d155e97afce49e5acb55af1692cc3
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

gerrit-hooks has posted comments on this change.

Change subject: infra: Use new concurrent.thread() utility
......................................................................


Patch Set 3:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6',
'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib7f76c4e7c6d155e97afce49e5acb55af1692cc3
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: infra: Use new concurrent.thread() utility
......................................................................


Patch Set 3: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib7f76c4e7c6d155e97afce49e5acb55af1692cc3
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

[Piotr Kliczewski]

Piotr Kliczewski has posted comments on this change.

Change subject: infra: Use new concurrent.thread() utility
......................................................................


Patch Set 3: Code-Review+1

(1 comment)

https://gerrit.ovirt.org/#/c/45554/3//COMMIT_MSG
Commit Message:

Line 15: - supervdsmServer server thread is protected from silent failures.
Line 16: 
Line 17: This patch does not update:
Line 18: 
Line 19: - lib/yajsonrpc - it is not clear if we want to make it depend on vdsm
yajsonrpc uses executor now. I think that we can skip this comment
Line 20:   library
Line 21: - debug plugin - being removed in https://gerrit.ovirt.org/44724
Line 22: 
Line 23: Change-Id: Ib7f76c4e7c6d155e97afce49e5acb55af1692cc3


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib7f76c4e7c6d155e97afce49e5acb55af1692cc3
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes