[CRIU] [PATCH 07/11] link-remap: open link-remap files from correct mountpoints

[Pavel Emelyanov]

On 07/25/2014 01:12 AM, Andrey Vagin wrote:
> Here is a problem with ghost files. Links are created on restore, but
> they can't be created on any mount point, because a mount point can be
> non-root bind-mount of another one. So we need to find the root mount
> and create all links there.
> 
> Signed-off-by: Andrey Vagin <avagin at openvz.org>

Can we split this patch into ghost and link-remap support?

> ---
>  files-reg.c         | 68 +++++++++++++++++++++++++++++++++++++++++++----------
>  include/files-reg.h |  1 +
>  2 files changed, 57 insertions(+), 12 deletions(-)
> 
> diff --git a/files-reg.c b/files-reg.c
> index d8875d6..b1cb78b 100644
> --- a/files-reg.c
> +++ b/files-reg.c
> @@ -73,8 +73,9 @@ static int create_ghost(struct ghost_file *gf, GhostFileEntry *gfe, char *root,
>  	int gfd, ghost_flags, ret = -1;
>  	char path[PATH_MAX];
>  
> +	snprintf(path, sizeof(path), "%s/%s", root, gf->remap.path);
>  	if (S_ISFIFO(gfe->mode)) {
> -		if (mknod(gf->remap.path, gfe->mode, 0)) {
> +		if (mknod(path, gfe->mode, 0)) {
>  			pr_perror("Can't create node for ghost file");
>  			goto err;
>  		}
> @@ -85,13 +86,13 @@ static int create_ghost(struct ghost_file *gf, GhostFileEntry *gfe, char *root,
>  			goto err;
>  		}
>  
> -		if (mknod(gf->remap.path, gfe->mode, gfe->rdev)) {
> +		if (mknod(path, gfe->mode, gfe->rdev)) {
>  			pr_perror("Can't create node for ghost dev");
>  			goto err;
>  		}
>  		ghost_flags = O_WRONLY;
>  	} else if (S_ISDIR(gfe->mode)) {
> -		if (mkdir(gf->remap.path, gfe->mode)) {
> +		if (mkdir(path, gfe->mode)) {
>  			pr_perror("Can't make ghost dir");
>  			goto err;
>  		}
> @@ -99,7 +100,6 @@ static int create_ghost(struct ghost_file *gf, GhostFileEntry *gfe, char *root,
>  	} else
>  		ghost_flags = O_WRONLY | O_CREAT | O_EXCL;
>  
> -	snprintf(path, sizeof(path), "%s/%s", root, gf->remap.path);
>  	gfd = open(path, ghost_flags, gfe->mode);
>  	if (gfd < 0) {
>  		pr_perror("Can't open ghost file %s", path);
> @@ -154,6 +154,7 @@ static int open_remap_ghost(struct reg_file_info *rfi,
>  	if (!gf)
>  		return -1;
>  	gf->remap.path = xmalloc(PATH_MAX);
> +	gf->remap.mnt_id = rfi->rfe->mnt_id;
>  	if (!gf->remap.path)
>  		goto err;
>  
> @@ -221,6 +222,7 @@ static int open_remap_linked(struct reg_file_info *rfi,
>  	rm->path = rrfi->path;
>  	rm->users = 0;
>  	rm->is_dir = false;
> +	rm->mnt_id = rfi->rfe->mnt_id;
>  	rfi->remap = rm;
>  	return 0;
>  }
> @@ -313,8 +315,12 @@ void remap_put(struct file_remap *remap)
>  {
>  	mutex_lock(ghost_file_mutex);
>  	if (--remap->users == 0) {
> +		int mntns_root;
> +
>  		pr_info("Unlink the ghost %s\n", remap->path);
> -		unlink(remap->path);
> +
> +		mntns_root = mntns_get_root_by_mnt_id(remap->mnt_id);

Isn't it better to save nsid on remap?

> +		unlinkat(mntns_root, remap->path, 0);
>  	}
>  	mutex_unlock(ghost_file_mutex);
>  }
> @@ -439,6 +445,7 @@ static int create_link_remap(char *path, int len, int lfd,
>  	rfe.pos		= 0;
>  	rfe.fown	= &fwn;
>  	rfe.name	= link_name + 1;
> +	rfe.has_mnt_id  = true;

What for?

>  
>  	/* Any 'unique' name works here actually. Remap works by reg-file ids. */
>  	snprintf(tmp + 1, sizeof(link_name) - (size_t)(tmp - link_name - 1), "link_remap.%d", rfe.id);
> @@ -669,7 +676,45 @@ const struct fdtype_ops regfile_dump_ops = {
>  
>  static inline int rfi_remap(struct reg_file_info *rfi)
>  {
> -	return link(rfi->remap->path, rfi->path);
> +	struct mount_info *mi, *rmi, *tmi;
> +	int off, roff;
> +	char path[PATH_MAX], rpath[PATH_MAX];
> +	int mntns_root;
> +
> +	mi = lookup_mnt_id(rfi->rfe->mnt_id);
> +
> +	if (mi == NULL) {
> +		mntns_root = mntns_get_root_by_mnt_id(-1);
> +		return linkat(mntns_root, rfi->remap->path, mntns_root, rfi->path, 0);
> +	}
> +
> +	/*
> +	 * mi->mountpoint	./zdtm/live/static/mntns_link_ghost.test/1
> +	 * + mi->rst_off	/zdtm/live/static/mntns_link_ghost.test/1
> +	 * rfi->path		zdtm/live/static/mntns_link_ghost.test/1/F (deleted)
> +	 */
> +	rmi = lookup_mnt_id(rfi->remap->mnt_id);
> +	off = strlen(mi->mountpoint + mi->rst_off + 1);
> +	roff = strlen(rmi->mountpoint + rmi->rst_off + 1);
> +
> +	/* Find the lowest common bind-mount */
> +	for (tmi = mi; tmi->bind; tmi = tmi->bind);
> +
> +	/* Create paths relative to this mount */
> +	snprintf(path, sizeof(path), "%s/%s/%s",
> +				tmi->mountpoint + tmi->rst_off + 1,
> +				mi->root + strlen(tmi->root),
> +				rfi->path + off);
> +	snprintf(rpath, sizeof(rpath), "%s/%s/%s",
> +				tmi->mountpoint + tmi->rst_off + 1,
> +				rmi->root + strlen(tmi->root),
> +				rfi->remap->path + roff);

I don't get this path tossing, can you be more specific?

> +
> +	pr_debug("%d: Link %s -> %s\n", tmi->mnt_id, path, rpath);
> +
> +	mntns_root = mntns_get_root_by_mnt_id(tmi->mnt_id);
> +
> +	return linkat(mntns_root, rpath, mntns_root, path, 0);
>  }
>  
>  int open_path(struct file_desc *d,
> @@ -752,16 +797,15 @@ int open_path(struct file_desc *d,
>  	}
>  
>  	if (rfi->remap) {
> -		if (!rfi->remap->is_dir)
> -			unlink(rfi->path);
> +		if (!rfi->remap->is_dir) {
> +			unlinkat(mntns_root, rfi->path, 0);
> +		}
>  
>  		BUG_ON(!rfi->remap->users);
>  		if (--rfi->remap->users == 0) {
>  			pr_info("Unlink the ghost %s\n", rfi->remap->path);
> -			if (rfi->remap->is_dir)
> -				rmdir(rfi->remap->path);
> -			else
> -				unlink(rfi->remap->path);
> +			mntns_root = mntns_get_root_by_mnt_id(rfi->remap->mnt_id);
> +			unlinkat(mntns_root, rfi->remap->path, rfi->remap->is_dir ? AT_REMOVEDIR : 0);
>  		}
>  
>  		if (orig_path)
> diff --git a/include/files-reg.h b/include/files-reg.h
> index b4a7367..bc5f5a7 100644
> --- a/include/files-reg.h
> +++ b/include/files-reg.h
> @@ -14,6 +14,7 @@ struct fd_parms;
>  struct file_remap {
>  	char *path;
>  	bool is_dir;
> +	int  mnt_id;
>  	unsigned int users;
>  };
>  
>