[CRIU] [PATCH 14/15] restore: restore pocesses which share one fdtable (v4)

[Andrew Vagin]

On Thu, Jan 10, 2013 at 03:34:50PM +0400, Pavel Emelyanov wrote:
> On 01/10/2013 02:00 PM, Andrey Vagin wrote:
> > Currenly crtools supports a case when a child shared a fd table
> > with parent.
> > 
> > Here is only two interesting things.
> > * Service descriptors should be cloned for each process
> >   who shared one fd table.
> > * One task should restore files and other tasks should sleep in this
> > * time.
> > 
> > v2: * allocate fdt_lock from shared memory
> >     * don't wait a child, if it doesn't share fdtable
> > v3: * don't move ids on the pstree image
> > v4: * save ids in a separate image
> >     * save fdinfo per id instead of pid
> > 
> > Signed-off-by: Andrey Vagin <avagin at openvz.org>
> > ---
> >  cr-dump.c    | 16 +++++++++-------
> >  cr-restore.c | 17 ++++++++++++++---
> >  files.c      | 29 +++++++++++++++++++++++------
> >  3 files changed, 46 insertions(+), 16 deletions(-)
> > 
> > diff --git a/cr-dump.c b/cr-dump.c
> > index ef66985..189e115 100644
> > --- a/cr-dump.c
> > +++ b/cr-dump.c
> > @@ -689,7 +689,7 @@ static int dump_task_kobj_ids(struct pstree_item *item)
> >  
> >  	new = 0;
> >  	ids->files_id = kid_generate_gen(&files_tree, &elem, &new);
> > -	if (!ids->files_id || !new) {
> > +	if (!ids->files_id || (!new && !shared_fdtable(item))) {
> >  		pr_err("Can't make FILES id for %d\n", pid);
> >  		return -1;
> >  	}
> > @@ -1425,12 +1425,6 @@ static int dump_one_task(struct pstree_item *item)
> >  	if (!cr_fdset)
> >  		goto err_cure;
> >  
> > -	ret = dump_task_files_seized(parasite_ctl, item, dfds);
> > -	if (ret) {
> > -		pr_err("Dump files (pid: %d) failed with %d\n", pid, ret);
> > -		goto err_cure;
> > -	}
> > -
> >  	ret = parasite_dump_pages_seized(parasite_ctl, &vma_area_list, cr_fdset);
> >  	if (ret) {
> >  		pr_err("Can't dump pages (pid: %d) with parasite\n", pid);
> > @@ -1462,6 +1456,14 @@ static int dump_one_task(struct pstree_item *item)
> >  		goto err_cure;
> >  	}
> >  
> > +	if (!shared_fdtable(item)) {
> 
> I've already asked to make shared_fdtable() check be not 6 ifs, but a bit
> on a pstree item.
Now I use shared_fdtable once for dumping and twice for restoring...
> 
> > +		ret = dump_task_files_seized(parasite_ctl, item, dfds);
> > +		if (ret) {
> > +			pr_err("Dump files (pid: %d) failed with %d\n", pid, ret);
> > +			goto err_cure;
> > +		}
> > +	}
> > +
> >  	ret = dump_task_threads(parasite_ctl, item);
> >  	if (ret) {
> >  		pr_err("Can't dump threads\n");
> > diff --git a/cr-restore.c b/cr-restore.c
> > index 26783a2..ffb1a10 100644
> > --- a/cr-restore.c
> > +++ b/cr-restore.c
> > @@ -805,6 +805,9 @@ static inline int fork_with_pid(struct pstree_item *item, unsigned long ns_clone
> >  	ca.item = item;
> >  	ca.clone_flags = ns_clone_flags;
> >  
> > +	if (shared_fdtable(item))
> > +		ca.clone_flags |= CLONE_FILES;
> > +
> >  	if (!(ca.clone_flags & CLONE_NEWPID)) {
> >  		char buf[32];
> >  
> > @@ -992,11 +995,19 @@ static int restore_task_with_children(void *_arg)
> >  	int ret;
> >  	sigset_t blockmask;
> >  
> > -	close_safe(&ca->fd);
> > -	close_old_fds(current);
> > -
> >  	current = ca->item;
> >  
> > +	if (ca->clone_flags & CLONE_FILES)
> > +		ret = clone_service_fd(false);
> > +	else {
> > +		close_safe(&ca->fd);
> > +		close_old_fds(current);
> > +		ret = clone_service_fd(true);
> > +	}
> > +
> > +	if (ret != 0)
> > +		return -1;
> > +
> >  	pid = getpid();
> >  	if (current->pid.virt != pid) {
> >  		pr_err("Pid %d do not match expected %d\n", pid, current->pid.virt);
> > diff --git a/files.c b/files.c
> > index 60d68f1..b30b5cf 100644
> > --- a/files.c
> > +++ b/files.c
> > @@ -534,17 +534,32 @@ int close_old_fds(struct pstree_item *me)
> >  	return 0;
> >  }
> >  
> > +extern struct pstree_item *current;
> 
> To header.
It should be removed...
> 
> >  int prepare_fds(struct pstree_item *me)
> >  {
> > -	u32 ret;
> > +	u32 ret = 0;
> >  	int state;
> >  
> > -	ret = close_old_fds(me);
> > -	if (ret)
> > -		goto err;
> > -
> >  	pr_info("Opening fdinfo-s\n");
> >  
> > +	if (me->rst->fdt) {
> > +		struct fdt *fdt = me->rst->fdt;
> > +
> > +		/*
> > +		 * Wait all tasks, who share a current fd table.
> > +		 * We should be sure, that nobody use any file
> > +		 * descriptor while fdtable is being restored.
> > +		 */
> > +		futex_inc_and_wake(&fdt->fdt_lock);
> > +		futex_wait_while_lt(&fdt->fdt_lock, fdt->nr);
> > +
> > +		if (fdt->pid != me->pid.virt) {
> > +			pr_info("File descriptor talbe is shared with %d\n", fdt->pid);
> > +			futex_wait_until(&fdt->fdt_lock, fdt->nr + 1);
> > +			goto out;
> > +		}
> > +	}
> > +
> >  	for (state = 0; state < ARRAY_SIZE(states); state++) {
> >  		ret = open_fdinfos(me->pid.virt, &me->rst->fds, state);
> >  		if (ret)
> > @@ -568,7 +583,9 @@ int prepare_fds(struct pstree_item *me)
> >  			break;
> >  	}
> >  
> > -err:
> > +	if (me->rst->fdt)
> > +		futex_inc_and_wake(&me->rst->fdt->fdt_lock);
> > +out:
> >  	tty_fini_fds();
> >  	return ret;
> >  }
> > 
> 
>