[lxc-devel] [PATCH 1/5] start child in its own process group, and put it into the foreground

[Matt Helsley]

On Wed, Jun 09, 2010 at 09:56:58PM +0200, Ferenc Wagner wrote:
> Matt Helsley <matthltc at us.ibm.com> writes:
> 
> > On Wed, Jun 09, 2010 at 07:56:03PM +0200, Ferenc Wagner wrote:
> >
> >> Signed-off-by: Ferenc Wagner <wferi at niif.hu>
> >> ---
> >>  src/lxc/start.c |   17 +++++++++++++++++
> >>  1 files changed, 17 insertions(+), 0 deletions(-)
> >> 
> >> diff --git a/src/lxc/start.c b/src/lxc/start.c
> >> index b69ac88..7bbcf5a 100644
> >> --- a/src/lxc/start.c
> >> +++ b/src/lxc/start.c
> >> @@ -463,6 +463,7 @@ int lxc_spawn(struct lxc_handler *handler)
> >>  	int clone_flags;
> >>  	int failed_before_rename = 0;
> >>  	const char *name = handler->name;
> >> +	int ctty;
> >> 
> >>  	if (lxc_sync_init(handler))
> >>  		return -1;
> >> @@ -509,6 +510,22 @@ int lxc_spawn(struct lxc_handler *handler)
> >>  		}
> >>  	}
> >> 
> >> +	if (setpgid(handler->pid, 0)) {
> >
> > I think this races with the exec in the child. From the setpgid() man page:
> >
> > ERRORS
> >        EACCES An attempt was made to change the process group ID of one of the
> >               children  of  the calling process and the child had already per‐
> >               formed an execve(2) (setpgid(), setpgrp()).
> >
> > You may be able to fix this by also doing setpgid() in the child before the
> > exec.
> >
> > Acked-by: Matt Helsley <matthltc at us.ibm.com>
> 
> Thanks for the review.  Isn't the race excluded by
> lxc_sync_barrier_child(handler, LXC_SYNC_POST_CONFIGURE) being on the
> following lines, not before?  Then I was misled by the comment before
> it.

Ahh, you're right! Looks good.

Cheers,
	-Matt Helsley