Nir Soffer has posted comments on this change.

Change subject: supervdsm: Add wait for SIGKILLed processes
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.ovirt.org/#/c/34142/1/vdsm/supervdsmServer
File vdsm/supervdsmServer:

Line 242:                 if e.errno != errno.ESRCH:
Line 243:                     raise
Line 244: 
Line 245:             # If we just killed a process or it died already, wait on 
it.
Line 246:             os.waitpid(proc.pid, os.WNOHANG)
> The process is dead at this point, and was not waited upon.
The process may be still alive - os.kill is async - right?

 1. You ask the kernel to kill the process
 2. Kernel say ok (but it did not kill the process yet)
 3. Then you wait 
 4. wait returns because process was not killed yet
 5. Kernel kill the process

If we don't have a problem to block here for couple of seconds we can do:

    for i in range(3):
        if os.waitpid(proc.pid, os.WNOHANG) == proc.pid:
            return
        time.sleep(1.0)

    log warning...
Line 247:             raise Timeout()
Line 248: 
Line 249:         res, err = pipe.recv()
Line 250:         pipe.send("Bye")


-- 
To view, visit http://gerrit.ovirt.org/34142
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id97a578f77cf2c9a98788b4c2e29a799b2784fc3
Gerrit-PatchSet: 1
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: Yeela Kaplan <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: mooli tayer <[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