[CRIU] [PATCH 1/2 v2] mnt: clean up a root yard after openning all files

Andrew Vagin avagin at virtuozzo.com
Sat Jul 16 18:50:08 PDT 2016


On Sat, Jul 16, 2016 at 11:28:12AM -0700, Andrew Vagin wrote:
> On Sat, Jul 16, 2016 at 12:51:49PM +0300, Pavel Emelyanov wrote:
> > On 07/16/2016 02:44 AM, Andrey Vagin wrote:
> > > From: Andrew Vagin <avagin at virtuozzo.com>
> > 
> > I don't get how subject correlates with the patch. The call to
> > depopulate_roots_yard is anyway at the end of restore, as it
> > was, why "after opening all files"?
> 
> Now depopulate_roots_yard() is called from the root task before
> finishing CR_STATE_FORKING.
> 
> I moved it to the criu process and do it after clean_remaps(), because
> clean_remaps() uses the roots yard.
> 
> It's called after openning all files, because only at this moment we can
> be sure that all link remap files can be removed.
> 

restore_task_with_children()		| restore_root_task()
-----------------------------------------------------------------------
depopulate_roots_yard()			|
restore_finish_stage(CR_STATE_FORKING)	|
prepare_fds()				|
open_vmas()				|
					| restore_switch_stage(CR_STATE_RESTORE_SIGCHLD)
					| clean_remaps = 0;

If something fails between CR_STATE_FORKING and CR_STATE_RESTORE_SIGCHLD,
try_clean_remaps will be called().

try_clean_remaps()
  try_clean_ghost()
    rst_get_mnt_root()
      print_ns_root()
	snprintf(buf, bs, "%s/%d", mnt_roots, ns->id);

it uses mnt_roots, actually it is what we called the roots yard.

> > 
> > > The root yard is used to clean up ghost files.
> > > 
> > > Now try_clean_remaps() is called from depopulate_roots_yard(), so
> > > the code about switching mount namespaces was moved to
> > > depopulate_roots_yard().
> > > 
> > > v2: call clean_remaps() when processes are restored in
> > >     the host mount namespace.
> > > 
> > > Signed-off-by: Andrew Vagin <avagin at virtuozzo.com>
> > > ---
> > >  criu/cr-restore.c        | 11 ++++-----
> > >  criu/files-reg.c         | 47 ++----------------------------------
> > >  criu/include/files-reg.h |  2 +-
> > >  criu/include/mount.h     |  2 +-
> > >  criu/mount.c             | 63 ++++++++++++++++++++++++++++++++++++++++++++++--
> > >  5 files changed, 70 insertions(+), 55 deletions(-)
> > > 
> > > diff --git a/criu/cr-restore.c b/criu/cr-restore.c
> > > index b4b7038..e1842b0 100644
> > > --- a/criu/cr-restore.c
> > > +++ b/criu/cr-restore.c
> > > @@ -80,6 +80,7 @@
> > >  #include "syscall-types.h"
> > >  #include "parasite-syscall.h"
> > >  #include "syscall-codes.h"
> > > +#include "files-reg.h"
> > > 
> > >  #include "protobuf.h"
> > >  #include "images/sa.pb-c.h"
> > > @@ -1438,9 +1439,6 @@ static int restore_task_with_children(void *_arg)
> > >  		 */
> > >  		futex_wait_while_gt(&task_entries->nr_in_progress, 1);
> > > 
> > > -		if (depopulate_roots_yard())
> > > -			goto err;
> > > -
> > >  		fini_restore_mntns();
> > >  	}
> > > 
> > > @@ -1882,9 +1880,11 @@ static int restore_root_task(struct pstree_item *init)
> > >  	 * There is no need to call try_clean_remaps() after this point,
> > >  	 * as restore went OK and all ghosts were removed by the openers.
> > >  	 */
> > > +	if (depopulate_roots_yard(mnt_ns_fd, false))
> > > +		goto out_kill;
> > > +
> > >  	clean_remaps = 0;
> > >  	close_safe(&mnt_ns_fd);
> > > -	cleanup_mnt_ns();
> > > 
> > >  	ret = stop_usernsd();
> > >  	if (ret < 0)
> > > @@ -1989,8 +1989,7 @@ out_kill:
> > >  out:
> > >  	fini_cgroup();
> > >  	if (clean_remaps)
> > > -		try_clean_remaps(mnt_ns_fd);
> > > -	cleanup_mnt_ns();
> > > +		depopulate_roots_yard(mnt_ns_fd, true);
> > >  	stop_usernsd();
> > >  	__restore_switch_stage(CR_STATE_FAIL);
> > >  	pr_err("Restoring FAILED.\n");
> > > diff --git a/criu/files-reg.c b/criu/files-reg.c
> > > index 157cb8f..67d1b6c 100644
> > > --- a/criu/files-reg.c
> > > +++ b/criu/files-reg.c
> > > @@ -541,60 +541,17 @@ static int clean_one_remap(struct file_remap *remap)
> > >  	return 0;
> > >  }
> > > 
> > > -void try_clean_remaps(int ns_fd)
> > > +void try_clean_remaps()
> > >  {
> > >  	struct remap_info *ri;
> > > -	int old_ns = -1;
> > > -	int cwd_fd = -1;
> > > 
> > >  	if (list_empty(&remaps))
> > > -		goto out;
> > > -
> > > -	if (ns_fd >= 0) {
> > > -		pr_info("Switching to new ns to clean ghosts\n");
> > > -
> > > -		old_ns = open_proc(PROC_SELF, "ns/mnt");
> > > -		if (old_ns < 0) {
> > > -			pr_perror("`- Can't keep old ns");
> > > -			return;
> > > -		}
> > > -
> > > -		cwd_fd = open(".", O_DIRECTORY);
> > > -		if (cwd_fd < 0) {
> > > -			pr_perror("Unable to open cwd");
> > > -			return;
> > > -		}
> > > -
> > > -		if (setns(ns_fd, CLONE_NEWNS) < 0) {
> > > -			close(old_ns);
> > > -			close(cwd_fd);
> > > -			pr_perror("`- Can't switch");
> > > -			return;
> > > -		}
> > > -	}
> > > +		return;
> > > 
> > >  	list_for_each_entry(ri, &remaps, list)
> > >  		if (ri->rfe->remap_type == REMAP_TYPE__GHOST)
> > >  			try_clean_ghost(ri);
> > > 
> > > -	if (old_ns >= 0) {
> > > -		if (setns(old_ns, CLONE_NEWNS) < 0)
> > > -			pr_perror("Fail to switch back!");
> > > -		close(old_ns);
> > > -	}
> > > -
> > > -	if (cwd_fd >= 0) {
> > > -		if (fchdir(cwd_fd)) {
> > > -			pr_perror("Unable to restore cwd");
> > > -			close(cwd_fd);
> > > -			return;
> > > -		}
> > > -		close(cwd_fd);
> > > -	}
> > > -
> > > -out:
> > > -	if (ns_fd >= 0)
> > > -		close(ns_fd);
> > >  }
> > > 
> > >  static struct collect_image_info remap_cinfo = {
> > > diff --git a/criu/include/files-reg.h b/criu/include/files-reg.h
> > > index dd4e7c0..6cc2454 100644
> > > --- a/criu/include/files-reg.h
> > > +++ b/criu/include/files-reg.h
> > > @@ -51,7 +51,7 @@ extern int collect_remaps_and_regfiles(void);
> > >  extern void delete_link_remaps(void);
> > >  extern void free_link_remaps(void);
> > >  extern int prepare_remaps(void);
> > > -extern void try_clean_remaps(int ns_fd);
> > > +extern void try_clean_remaps(void);
> > > 
> > >  extern int strip_deleted(struct fd_link *link);
> > > 
> > > diff --git a/criu/include/mount.h b/criu/include/mount.h
> > > index c7992ac..d3f472d 100644
> > > --- a/criu/include/mount.h
> > > +++ b/criu/include/mount.h
> > > @@ -120,7 +120,7 @@ extern bool phys_stat_dev_match(dev_t st_dev, dev_t phys_dev,
> > > 
> > >  extern int restore_task_mnt_ns(struct pstree_item *current);
> > >  extern void fini_restore_mntns(void);
> > > -extern int depopulate_roots_yard(void);
> > > +extern int depopulate_roots_yard(int mntns_root, bool clean_remaps);
> > > 
> > >  extern int rst_get_mnt_root(int mnt_id, char *path, int plen);
> > >  extern int ext_mount_add(char *key, char *val);
> > > diff --git a/criu/mount.c b/criu/mount.c
> > > index 6512f51..7c280c0 100644
> > > --- a/criu/mount.c
> > > +++ b/criu/mount.c
> > > @@ -30,6 +30,7 @@
> > >  #include "sysfs_parse.h"
> > >  #include "path.h"
> > >  #include "autofs.h"
> > > +#include "files-reg.h"
> > > 
> > >  #include "images/mnt.pb-c.h"
> > >  #include "images/binfmt-misc.pb-c.h"
> > > @@ -3254,7 +3255,7 @@ static int populate_mnt_ns(void)
> > >  	return ret;
> > >  }
> > > 
> > > -int depopulate_roots_yard(void)
> > > +static int __depopulate_roots_yard(void)
> > >  {
> > >  	int ret = 0;
> > > 
> > > @@ -3273,8 +3274,66 @@ int depopulate_roots_yard(void)
> > >  	 */
> > >  	if (umount2(mnt_roots, MNT_DETACH)) {
> > >  		pr_perror("Can't unmount %s", mnt_roots);
> > > -		ret = 1;
> > > +		ret = -1;
> > > +	}
> > > +
> > > +	if (rmdir(mnt_roots)) {
> > > +		pr_perror("Can't remove the directory %s", mnt_roots);
> > > +		ret = -1;
> > > +	}
> > > +
> > > +	return ret;
> > > +}
> > > +
> > > +int depopulate_roots_yard(int mntns_fd, bool clean_remaps)
> > > +{
> > > +	int ret = 0, old_cwd = -1, old_ns = -1;
> > > +
> > > +	if (mntns_fd < 0) {
> > > +		if (clean_remaps)
> > > +			try_clean_remaps();
> > > +		cleanup_mnt_ns();
> > > +		return 0;
> > > +	}
> > > +
> > > +	pr_info("Switching to new ns to clean ghosts\n");
> > > +
> > > +	old_cwd = open(".", O_PATH);
> > > +	if (old_cwd < 0) {
> > > +		pr_perror("Unable to open cwd");
> > > +		return -1;
> > > +	}
> > > +
> > > +	old_ns = open_proc(PROC_SELF, "ns/mnt");
> > > +	if (old_ns < 0) {
> > > +		pr_perror("`- Can't keep old ns");
> > > +		close(old_cwd);
> > > +		return -1;
> > > +	}
> > > +	if (setns(mntns_fd, CLONE_NEWNS) < 0) {
> > > +		close(old_ns);
> > > +		close(old_cwd);
> > > +		pr_perror("`- Can't switch");
> > > +		return -1;
> > > +	}
> > > +
> > > +	if (clean_remaps)
> > > +		try_clean_remaps();
> > > +
> > > +	if (__depopulate_roots_yard())
> > > +		ret = -1;
> > > +
> > > +	if (setns(old_ns, CLONE_NEWNS) < 0) {
> > > +		pr_perror("Fail to switch back!");
> > > +		ret = -1;
> > > +	}
> > > +	close(old_ns);
> > > +
> > > +	if (fchdir(old_cwd)) {
> > > +		pr_perror("Unable to restore cwd");
> > > +		ret = -1;
> > >  	}
> > > +	close(old_cwd);
> > > 
> > >  	return ret;
> > >  }
> > > --
> > > 2.7.4
> > > 
> > > .
> > > 
> > 


More information about the CRIU mailing list