Change in vdsm[master]: supervdsm: Add zombiereaper to supervdsm

danken at redhat.com danken at redhat.com
Thu Aug 14 22:17:14 UTC 2014


Dan Kenigsberg has posted comments on this change.

Change subject: supervdsm: Add zombiereaper to supervdsm
......................................................................


Patch Set 11:

(1 comment)

http://gerrit.ovirt.org/#/c/28915/11/vdsm/supervdsmServer
File vdsm/supervdsmServer:

Line 255:             if err is not None:
Line 256:                 raise err
Line 257: 
Line 258:             return res
Line 259:         finally:
I am shocked about the unpredictable semantics of terminate(). Do you understand why it waits only if kill() failed?

I think that we should take care to separate between the positive flow (process waited upon) and the negative one (process not yet collected), and auto-reap only the latter case.

Auto-reaping an already-collected pid has a slim race: proc.pid may already belong to another child process. Yes, it's not probable that pids would recycle so quickly, and would be re-used by another child process of supervdsm. But still, it's plain ugly.

I think that placing

  if proc.returncode is None:
     zombiereaper.autoReapPID(proc.pid)

in the except clause, would do the trick (but please verify).
Line 260:             zombiereaper.autoReapPID(proc.pid)
Line 261: 
Line 262:     @logDecorator
Line 263:     def validateAccess(self, user, groups, *args, **kwargs):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5333caf7a1162c5d783f556bb9204fb74d1ab469
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dima Kuznetsov <dkuznets at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Dima Kuznetsov <dkuznets at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list