Zhou Zheng Sheng has posted comments on this change.
Change subject: integrate zombie reaper in supervdsmServer
......................................................................
Patch Set 1: I would prefer that you didn't submit this
(3 inline comments)
....................................................
Commit Message
Line 9: Integrate zombie reaper in supervdsmServer
Line 10: to reclaim the zombie produced by validateAccess.
Line 11: As python signal handlers only occur between atomic
Line 12: python intepreter instructions, join thread will result in
Line 13: SIGCHLD infinite delay.Change it to a timeout join.
Actually, there are two things described here. Maybe a separate paragraph for
each change? Putting them together makes me confused for some seconds. After
reading the code, I get to know what it mean. Also, for the Python bug of
join() blocking the signal handlers, can you paste a reference link below?
Line 14:
Line 15: Change-Id: Idccc34d8761fb9997cda1184552c6c8f633afbf0
....................................................
File vdsm/supervdsmServer.py
Line 184: pipe.recv()
Line 185:
Line 186: pipe, hisPipe = Pipe()
Line 187: proc = Process(target=child, args=(hisPipe,))
Line 188: proc.start()
(2) It seems that "zombieReaper.autoReapPID(proc.pid)" can be added here.
Line 189:
Line 190: if not pipe.poll(RUN_AS_TIMEOUT):
Line 191: try:
Line 192: os.kill(proc.pid, signal.SIGKILL)
Line 194: # If it didn't fail because process is already dead
Line 195: if e.errno != errno.ESRCH:
Line 196: raise
Line 197:
Line 198: raise Timeout()
(1) If the process triggers timeout, the function returns early with the
timeout exception, so it will not run the rest of the code for registering the
pid to zombie reaper. I think the registration code can be added at the place
of (2)
Line 199:
Line 200: res, err = pipe.recv()
Line 201: pipe.send("Bye")
Line 202: proc.terminate()
--
To view, visit http://gerrit.ovirt.org/9315
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Idccc34d8761fb9997cda1184552c6c8f633afbf0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <[email protected]>
Gerrit-Reviewer: Saggi Mizrahi <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: Zhou Zheng Sheng <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
