[CRIU] [PATCH 4/5] files: Cleanup ghost subdirectories

[Andrei Vagin]

On Tue, Feb 26, 2019 at 12:13:34PM +0300, Cyrill Gorcunov wrote:
> In case if some of ghost directories has transient entries
> which we had to creat while restoring they cause inability
> to remove toplevel directory due to ENOTEMPTY error raised.
> 
> To address this we do the following:
> 
>  - when ghost entries are created remember which
>    dentries were created in @remap->subdirs
> 
>  - when we're cleaning them up we remove these
>    transient entries up to @remap->subdirs
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov at gmail.com>
> ---
>  criu/files-reg.c         | 51 ++++++++++++++++++++++++++++++++++++----
>  criu/include/files-reg.h |  2 ++
>  2 files changed, 49 insertions(+), 4 deletions(-)
> 
> diff --git a/criu/files-reg.c b/criu/files-reg.c
> index a36b60142760..f304efa8c770 100644
> --- a/criu/files-reg.c
> +++ b/criu/files-reg.c
> @@ -355,6 +355,7 @@ static int ghost_apply_metadata(const char *path, GhostFileEntry *gfe)
>  
>  static int create_ghost(struct ghost_file *gf, GhostFileEntry *gfe, struct cr_img *img)
>  {
> +	const char *subdir = NULL;
>  	struct mount_info *mi;
>  	char path[PATH_MAX];
>  	int ret, root_len;
> @@ -391,7 +392,8 @@ static int create_ghost(struct ghost_file *gf, GhostFileEntry *gfe, struct cr_im
>  		if ((ret = mknod(path, gfe->mode, gfe->rdev)) < 0)
>  			msg = "Can't create node for ghost dev";
>  	} else if (S_ISDIR(gfe->mode)) {
> -		if ((ret = mkdirpat(AT_FDCWD, path, gfe->mode)) < 0)
> +		ret = mkdirpat_precise(AT_FDCWD, path, gfe->mode, &subdir);
> +		if (ret < 0)
>  			msg = "Can't make ghost dir";
>  	} else {
>  		if ((ret = mkreg_ghost(path, gfe, img)) < 0)
> @@ -415,6 +417,14 @@ static int create_ghost(struct ghost_file *gf, GhostFileEntry *gfe, struct cr_im
>  	strcpy(gf->remap.rpath, path + root_len);
>  	pr_debug("Remap rpath is %s\n", gf->remap.rpath);
>  
> +	/*
> +	 * Save nested dirs for better cleanup.
> +	 */
> +	if (subdir && subdir != path) {
> +		gf->remap.subdir  = gf->remap.rpath;
> +		gf->remap.subdir += subdir - path - root_len;
> +	}
> +
>  	ret = -1;
>  	if (ghost_apply_metadata(path, gfe))
>  		goto err;
> @@ -758,6 +768,9 @@ static int order_remap_dirs(void)
>  			pr_debug("remap: ghost ord %3s %s\n",
>  				 ri->rfi->remap->is_dir ? "dir" : "fil",
>  				 ri->rfi->path);
> +			if (ri->rfi->remap->subdir)
> +				pr_debug("remap:\tnew subdir %s\n",
> +					 ri->rfi->remap->subdir);
>  		}
>  	}
>  
> @@ -783,6 +796,31 @@ int prepare_remaps(void)
>  	return ret ? : order_remap_dirs();
>  }
>  
> +static int cleanup_subdir(int dirfd, struct file_remap *remap)
> +{
> +	char *pos;
> +	int ret;
> +
> +	if (!remap->subdir)
> +		return 0;
> +
> +	/*
> +	 * Note that we're modifying @remap->rpath but
> +	 * caller is dropping this variable a bit later
> +	 * anyway!
> +	 */
> +	for (pos = strrchr(remap->subdir, '/'); pos;
> +	     pos = strrchr(pos, '/')) {
> +		*pos = '\0';
> +		pr_debug("\tUnlink remap subdir %s\n", remap->rpath);
> +		ret = unlinkat(dirfd, remap->rpath, AT_REMOVEDIR);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
>  static int clean_ghost_dir(int rmntns_root, struct remap_info *ri)
>  {
>  	struct file_remap *remap = ri->rfi->remap;
> @@ -791,10 +829,15 @@ static int clean_ghost_dir(int rmntns_root, struct remap_info *ri)
>  	 * When deleting ghost directories here is two issues:
>  	 * - names might duplicate, so we may receive ENOENT
>  	 *   and should not treat it as an error
> +	 * - some ghost directories might have transient entries
> +	 *   which we have to make during former path restoration
>  	 */
> -	if (ret)
> -		return errno == ENOENT ? 0 : -errno;
> -	return 0;
> +	if (ret) {
> +		if (errno == ENOENT)
> +			ret = 0;
> +	} else
> +		ret = cleanup_subdir(rmntns_root, remap);

should we call cleanup_subdir() if errno == ENOENT?
> +	return ret;
>  }
>  
>  static int clean_one_remap(struct remap_info *ri)
> diff --git a/criu/include/files-reg.h b/criu/include/files-reg.h
> index ec142db8c882..9b1339353878 100644
> --- a/criu/include/files-reg.h
> +++ b/criu/include/files-reg.h
> @@ -10,6 +10,7 @@ struct cr_imgset;
>  struct fd_parms;
>  
>  struct file_remap {
> +	char *subdir;
>  	char *rpath;
>  	bool is_dir;
>  	int  rmnt_id;
> @@ -20,6 +21,7 @@ struct file_remap {
>  static inline void file_remap_init(struct file_remap *remap)
>  {
>  	remap->rpath	= NULL;
> +	remap->subdir	= NULL;
>  	remap->is_dir	= false;
>  	remap->uid	= -1;
>  	remap->gid	= -1;
> -- 
> 2.20.1
> 
> _______________________________________________
> CRIU mailing list
> CRIU at openvz.org
> https://lists.openvz.org/mailman/listinfo/criu