Change in vdsm[master]: signals: Handle signals to non-main threads

nsoffer at redhat.com nsoffer at redhat.com
Thu Oct 30 07:07:38 UTC 2014


Nir Soffer has posted comments on this change.

Change subject: signals: Handle signals to non-main threads
......................................................................


Patch Set 16:

(3 comments)

http://gerrit.ovirt.org/#/c/29392/16/lib/vdsm/sigutils.py
File lib/vdsm/sigutils.py:

Line 18: # Refer to the README and COPYING files for full details of the license
Line 19: #
Line 20: 
Line 21: import errno
Line 22: import logging
Since both logs here are not needed, you can remove this import.
Line 23: import os
Line 24: import select
Line 25: import signal
Line 26: 


Line 89:     try:
Line 90:         cleanup = [] != _signal_poller.poll(timeout)
Line 91:     except select.error as e:
Line 92:         if e[0] != errno.EINTR:
Line 93:             logging.exception('Received error while waiting for signal')
No need to log if you raise. You did everything you can so the caller have the complete stack trace. If it ignore the exception it is the caller problem.
Line 94:             raise
Line 95:         cleanup = True
Line 96: 
Line 97:     if cleanup:


Line 117:     try:
Line 118:         while len(_read(_signal_read_fd, 128)) == 128:
Line 119:             pass
Line 120:     except OSError:
Line 121:         logging.exception('Received error while reading from pipe')
Again no need to log when you raise - in particular no an exception. Why do you want to have 2 backtraces in the log for this error?


-- 
To view, visit http://gerrit.ovirt.org/29392
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5dbcd00cec22ef12f2b6253b016dcbd0aa889583
Gerrit-PatchSet: 16
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dima Kuznetsov <dkuznets at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Dima Kuznetsov <dkuznets at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: mooli tayer <mtayer at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list