Nir Soffer has posted comments on this change. Change subject: signals: Handle signals to non-main threads ......................................................................
Patch Set 5: (3 comments) 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 think using multiprocessing simplifies a lot of things. We don't have to The only think we need to import is the sigutils file, so it is trivial to setup this in the child. I don't understand why we need to care about shared variables and pipes, subprocess module already handle pipes for us. We don't want our tests to fail because of less understood corners in multiprocessing library. 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') Line 52: write_pipe.send('two') Line 53: Line 54: proc = mp.Process(target=child_process) Line 55: proc.start() Line 56: self._waitForPoll(proc.pid) > Well, what I want to wait for, is for process to trigger wait_for_signal() If you write "ready" in the child, and then call wait_for_signals, and in the parent you read this value and wait 0.1 seconds, this is good enough. What you do is save 0.1 second sleep by assuming the implementation of wait_for_signal. We don't want to go in this direction. Line 57: os.kill(proc.pid, signal.SIGUSR1) Line 58: self.assertEquals(read_pipe.recv(), 'one') Line 59: proc.join() Line 60: self.assertEquals(read_pipe.recv(), 'two') Line 81: ctr = [0] Line 82: Line 83: def sighandler(*_): Line 84: ctr[0] += 1 Line 85: write_pipe.send('one') lets also replace these strings ("one", "two") with something more meaningful like "got signal" and "done" Line 86: Line 87: signal.signal(signal.SIGUSR1, sighandler) Line 88: while ctr[0] < TIMES: Line 89: sigutils.wait_for_signal() -- 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
