Yaniv Bronhaim has posted comments on this change.

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


Patch Set 5: Code-Review+1

(1 comment)

from my review it looks quite good and nice. please address nir's comments.

saggi, can you also review the sigutils part?

http://gerrit.ovirt.org/#/c/29392/5/tests/signalTests.py
File tests/signalTests.py:

Line 50:                           lambda *_: write_pipe.send('one'))
Line 51:             sigutils.wait_for_signal()
Line 52:             write_pipe.send('two')
Line 53: 
Line 54:         proc = mp.Process(target=child_process)
> I would not use multiprocessing, this just add uneeded complication and dep
its not really needed for a test
Line 55:         proc.start()
Line 56:         self._waitForPoll(proc.pid)
Line 57:         os.kill(proc.pid, signal.SIGUSR1)
Line 58:         self.assertEquals(read_pipe.recv(), 'one')


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