Dima Kuznetsov has posted comments on this change. Change subject: signals: Handle signals to non-main threads ......................................................................
Patch Set 4: (2 comments) 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 descript You're right! Adding. 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. It is true there are some quirks with poll returning while nothing can be read from the fd, but in that case read would return with EAGAIN, in other cases it may return with EINTR and I'll add a check to avoid those logs, but in all other cases I think it'd be wise to log the issue. Anyway, try/except clause moved directly around the read. -- 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
