[CRIU] [PATCH] restore: add action scripts setup-namespaces (v2)

[Andrew Vagin]

On Fri, May 31, 2013 at 04:37:55PM +0400, Pavel Emelyanov wrote:
> On 05/31/2013 02:29 PM, Andrey Vagin wrote:
> > After creating namespaces we may need to apply some configuration.
> > For example uid and gid maps should be applied in this moment.
> > 
> > This script should be executed from crtools, but namespaces are created
> > in a root process, so we need to add a new stage for synchronizing.
> > 
> > v2: use a separate stage instead of socketpair.
> > 
> > Signed-off-by: Andrey Vagin <avagin at openvz.org>
> > ---
> >  cr-restore.c       | 46 ++++++++++++++++++++++++++++++++++++++++++----
> >  include/restorer.h |  1 +
> >  2 files changed, 43 insertions(+), 4 deletions(-)
> > 
> > diff --git a/cr-restore.c b/cr-restore.c
> > index 70c5e68..8983f4c 100644
> > --- a/cr-restore.c
> > +++ b/cr-restore.c
> > @@ -931,6 +931,14 @@ static void sigchld_handler(int signal, siginfo_t *siginfo, void *data)
> >  
> >  	exit = (siginfo->si_code == CLD_EXITED);
> >  	status = siginfo->si_status;
> > +
> > +	/* skip scripts */
> > +	if (!current && root_item->pid.real != pid) {
> > +		pid = waitpid(root_item->pid.real, &status, WNOHANG);
> 
> Shouldn't while () loop be here?

Not, because crtools forks only a root task and SIGCHLD is blocked while
we are in sigchld hangler.

> 
> > +		if (pid <= 0)
> > +			return;
> > +	}
> > +
> >  	if (!current || status)
> >  		goto err;
> >  
> > @@ -1091,6 +1099,8 @@ static int restore_task_with_children(void *_arg)
> >  		if (mount_proc())
> >  			exit(1);
> >  
> > +		restore_finish_stage(CR_STATE_RESTORE_NS);
> > +
> >  		if (root_prepare_shared())
> >  			exit(1);
> >  	}
> > @@ -1158,6 +1168,8 @@ static int restore_task_with_children(void *_arg)
> >  static inline int stage_participants(int next_stage)
> >  {
> >  	switch (next_stage) {
> > +	case CR_STATE_RESTORE_NS:
> > +		return 1;
> >  	case CR_STATE_FORKING:
> >  		return task_entries->nr_tasks + task_entries->nr_helpers;
> >  	case CR_STATE_RESTORE_PGID:
> > @@ -1173,7 +1185,7 @@ static inline int stage_participants(int next_stage)
> >  	return -1;
> >  }
> >  
> > -static int restore_switch_stage(int next_stage)
> > +static int restore_wait_inprogress_tasks()
> >  {
> >  	int ret;
> >  	futex_t *np = &task_entries->nr_in_progress;
> > @@ -1183,7 +1195,19 @@ static int restore_switch_stage(int next_stage)
> >  	if (ret < 0)
> >  		return ret;
> >  
> > -	futex_set(np, stage_participants(next_stage));
> > +	return 0;
> > +}
> > +
> > +static int restore_switch_stage(int next_stage)
> > +{
> > +	int ret;
> > +
> > +	ret = restore_wait_inprogress_tasks();
> > +	if (ret)
> > +		return ret;
> > +
> > +	futex_set(&task_entries->nr_in_progress,
> > +			stage_participants(next_stage));
> >  	futex_set_and_wake(&task_entries->start, next_stage);
> >  	return 0;
> >  }
> > @@ -1230,12 +1254,26 @@ static int restore_root_task(struct pstree_item *init)
> >  		return -1;
> >  	}
> >  
> > -	futex_set(&task_entries->nr_in_progress, stage_participants(CR_STATE_FORKING));
> > +	futex_set(&task_entries->nr_in_progress,
> > +			stage_participants(CR_STATE_RESTORE_NS));
> >  
> >  	ret = fork_with_pid(init);
> >  	if (ret < 0)
> >  		return -1;
> >  
> > +	pr_info("Wait until namespaces are created\n");
> > +	ret = restore_wait_inprogress_tasks();
> > +	if (ret)
> > +		goto out;
> > +
> > +	ret = run_scripts("setup-namespaces");
> > +	if (ret)
> > +		goto out;
> > +
> > +	ret = restore_switch_stage(CR_STATE_FORKING);
> 
> This will call "wait-inprogress" again. Can we tune this thing out?

Do we need that? It will not call any syscall, just check that
nr_inprocess is zero.

> 
> > +	if (ret < 0)
> > +		goto out;
> > +
> >  	pr_info("Wait until all tasks are forked\n");
> >  	ret = restore_switch_stage(CR_STATE_RESTORE_PGID);
> >  	if (ret < 0)
> > @@ -1303,7 +1341,7 @@ static int prepare_task_entries()
> >  	task_entries->nr_threads = 0;
> >  	task_entries->nr_tasks = 0;
> >  	task_entries->nr_helpers = 0;
> > -	futex_set(&task_entries->start, CR_STATE_FORKING);
> > +	futex_set(&task_entries->start, CR_STATE_RESTORE_NS);
> >  	mutex_init(&task_entries->zombie_lock);
> >  
> >  	return 0;
> > diff --git a/include/restorer.h b/include/restorer.h
> > index caee41d..a5c0667 100644
> > --- a/include/restorer.h
> > +++ b/include/restorer.h
> > @@ -162,6 +162,7 @@ struct shmems {
> >  #define TASK_ENTRIES_SIZE 4096
> >  
> >  enum {
> > +	CR_STATE_RESTORE_NS, /* is used for executing "setup-namespace" scripts */
> >  	CR_STATE_FORKING,
> >  	CR_STATE_RESTORE_PGID,
> >  	CR_STATE_RESTORE,
> > 
> 
>