[CRIU] [PATCH 2/2] remap: add a dead pid /proc remap

[Andrew Vagin]

On Fri, Sep 05, 2014 at 02:38:05PM -0500, Tycho Andersen wrote:
> If a file like /proc/20/mountinfo is open, but 20 is a zombie (or doesn't exist
> any more), we can't read this file at all, so a link remap won't work. Instead,
> we add a new remap, called the dead process remap, which forks a TASK_HELPER as
> that dead pid so that the restore task can open the new /proc/20/mountinfo
> instead.

I would like to have a test case in the zdtm test suite for this case.

> 
> Signed-off-by: Tycho Andersen <tycho.andersen at canonical.com>
> ---
>  files-reg.c                    | 73 +++++++++++++++++++++++++++++++++++++++++-
>  include/fs-magic.h             |  4 +++
>  protobuf/remap-file-path.proto |  5 +++
>  3 files changed, 81 insertions(+), 1 deletion(-)
> 
> diff --git a/files-reg.c b/files-reg.c
> index 0ffce0e..eadd15f 100644
> --- a/files-reg.c
> +++ b/files-reg.c
> @@ -24,6 +24,7 @@
>  #include "asm/atomic.h"
>  #include "namespaces.h"
>  #include "proc_parse.h"
> +#include "pstree.h"
>  
>  #include "protobuf.h"
>  #include "protobuf/regfile.pb-c.h"
> @@ -227,6 +228,28 @@ static int open_remap_linked(struct reg_file_info *rfi,
>  	return 0;
>  }
>  
> +static int open_remap_dead_process(struct reg_file_info *rfi,
> +		RemapFilePathEntry *rfe)
> +{
> +	struct pstree_item *helper;
> +
> +	helper = alloc_pstree_item_with_rst();
> +	if (!helper)
> +		return -1;
> +
> +	helper->sid = root_item->sid;
> +	helper->pgid = root_item->sid;
> +	helper->pid.virt = rfe->remap_id;
> +	helper->state = TASK_HELPER;
> +	helper->parent = root_item;
> +	list_add_tail(&helper->sibling, &root_item->children);
> +	task_entries->nr_helpers++;
> +
> +	pr_info("Added a helper for restoring /proc/%d\n", helper->pid.virt);
> +
> +	return 0;
> +}
> +
>  static int collect_one_remap(void *obj, ProtobufCMessage *msg)
>  {
>  	int ret = -1;
> @@ -248,8 +271,10 @@ static int collect_one_remap(void *obj, ProtobufCMessage *msg)
>  
>  	if (rfe->remap_id & REMAP_GHOST)
>  		ret = open_remap_ghost(rfi, rfe);
> -	else
> +	else if (!rfe->has_remap_type)

I think we should have a separate type for this case.

>  		ret = open_remap_linked(rfi, rfe);
> +	else if (rfe->remap_type == REMAP_TYPE__PROCFS)
> +		ret = open_remap_dead_process(rfi, rfe);
>  out:
>  	return ret;
>  }
> @@ -498,6 +523,20 @@ static int dump_linked_remap(char *path, int len, const struct stat *ost,
>  			&rpe, PB_REMAP_FPATH);
>  }
>  
> +static int dump_dead_process_remap(pid_t pid, char *path, int len, const struct stat *ost,
> +				int lfd, u32 id, struct ns_id *nsid)
> +{
> +	RemapFilePathEntry rpe = REMAP_FILE_PATH_ENTRY__INIT;
> +
> +	rpe.orig_id = id;
> +	rpe.remap_id = pid;
> +	rpe.has_remap_type = true;
> +	rpe.remap_type = REMAP_TYPE__PROCFS;
> +
> +	return pb_write_one(fdset_fd(glob_fdset, CR_FD_REMAP_FPATH),
> +			&rpe, PB_REMAP_FPATH);
> +}
> +
>  static bool is_sillyrename_name(char *name)
>  {
>  	int i;
> @@ -539,6 +578,38 @@ static int check_path_remap(char *rpath, int plen, const struct fd_parms *parms,
>  	struct stat pst;
>  	const struct stat *ost = &parms->stat;
>  
> +	if (parms->fs_type == PROC_SUPER_MAGIC) {
> +		/* The file points to /proc/pid/<foo> where pid is a dead
> +		 * process. We remap this file by adding this pid to be
> +		 * fork()ed into a TASK_HELPER state so that we can point to it
> +		 * on restore.
> +		 */
> +		pid_t pid;
> +		char *start, *end;
> +
> +		/* skip "./proc/" */
> +		start = strstr(rpath, "/") + 1;
> +		if (!start)
> +			return -1;
> +		start = strstr(start, "/") + 1;
> +		if (!start)
> +			return -1;
> +		pid = strtol(start, &end, 10);
> +
> +		/* if we didn't find another /, this path something
> +		 * like ./proc/kmsg, which we shouldn't mess with. */
> +		if (*end == '/') {
> +			*end = 0;
> +			ret = access(rpath, F_OK);
> +			*end = '/';
> +
> +			if (ret) {
> +				pr_info("Dumping dead process remap of %d\n", pid);
> +				return dump_dead_process_remap(pid, rpath + 1, plen - 1, ost, lfd, id, nsid);
> +			}
> +		}
> +	}
> +
>  	if (ost->st_nlink == 0)
>  		/*
>  		 * Unpleasant, but easy case. File is completely invisible
> diff --git a/include/fs-magic.h b/include/fs-magic.h
> index 13c4961..777d736 100644
> --- a/include/fs-magic.h
> +++ b/include/fs-magic.h
> @@ -41,4 +41,8 @@
>  #define AUFS_SUPER_MAGIC	0x61756673
>  #endif
>  
> +#ifndef PROC_SUPER_MAGIC
> +#define PROC_SUPER_MAGIC	0x9fa0
> +#endif
> +
>  #endif /* __CR_FS_MAGIC_H__ */
> diff --git a/protobuf/remap-file-path.proto b/protobuf/remap-file-path.proto
> index faeb745..294cdec 100644
> --- a/protobuf/remap-file-path.proto
> +++ b/protobuf/remap-file-path.proto
> @@ -1,4 +1,9 @@
> +enum remap_type {
Can we add a type for linked files? And I think its value should be 0.

	LINKED			= 0;
	PROCFS                  = 1;

> +	PROCFS			= 0;
> +};
> +
>  message remap_file_path_entry {
>  	required uint32		orig_id		= 1;
>  	required uint32		remap_id	= 2;
> +	optional uint32		remap_type	= 3;
>  }
> -- 
> 1.9.1
> 
> _______________________________________________
> CRIU mailing list
> CRIU at openvz.org
> https://lists.openvz.org/mailman/listinfo/criu