Nir Soffer has posted comments on this change. Change subject: signals: Handle signals to non-main threads ......................................................................
Patch Set 4: Code-Review-1 (2 comments) Can we also have couple of tests? I think this should be quite easy by running a trivial script using this utility for waiting for signals. http://gerrit.ovirt.org/#/c/29392/4/lib/vdsm/sigutils.py File lib/vdsm/sigutils.py: Line 48: utils.set_non_blocking(write_fd) Line 49: Line 50: # Set the read pipe end to non-blocking too, just in case. Line 51: utils.set_non_blocking(read_fd) Line 52: It will also be a good idea to use utils.closeOnExec on these file descriptors. There is no reason that a child process will inherit them. We do the same in protocol detector. Line 53: # This call must be done from the main thread. Line 54: signal.set_wakeup_fd(write_fd) Line 55: Line 56: poller = select.poll() Line 81: if (e[0] != errno.EINTR): Line 82: logging.exception('Received error while waiting for signal') Line 83: except OSError as e: Line 84: if (e.errno != errno.EAGAIN): Line 85: logging.exception('Received error while reading from pipe') This is related only to os.read, so lets check this only around this call. When this happens, we should not log any exception or error - this is expected (if unlikely) condition. Nobody promise you that a file will be readable when poll returns, this is only a suggestion. If you think that it may help to debug issues, you log a debug message in this place. -- 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: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dima Kuznetsov <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Dima Kuznetsov <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Saggi Mizrahi <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
