Nir Soffer has posted comments on this change.

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


Patch Set 7:

(2 comments)

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

Line 28: 
Line 29: _signal_poller = None
Line 30: 
Line 31: 
Line 32: def _get_signal_poller():
> trivia bit: if user calls register but never actually reads from the read e
This is what python is doing:

    // Modules/signalmodule.c
    181         if (wakeup_fd != -1)
    182             write(wakeup_fd, "\0", 1);

If the pipe is full, write will fail with EAGAIN, and python would continue 
without any error, since it does not care if the user is  reading from the 
other end or not, and the kernel does not care about it either.
Line 33:     '''
Line 34:     This function creates a select.poll object that can be used in the 
same
Line 35:     manner as signal.pause(). The poll object returns each time a 
signal was
Line 36:     received by the process.


Line 87:                         logging.exception(
Line 88:                             'Received error while reading from pipe')
Line 89:     except select.error as e:
Line 90:         if e[0] != errno.EINTR:
Line 91:             logging.exception('Received error while waiting for 
signal')
> I liked this idea when we discussed it at first, but then I realized it sti
The important behavior is to return only when a signal was received, and return 
once for each signal.

You can implement it as you like, but I suggest you use the way that require 
less code, since we have enough code to maintain.

Using more efficient code is nice property but not very important unless you 
can show in a profile that this code is a bottleneck.


-- 
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: 7
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

Reply via email to