[CRIU] [PATCH 1/6] files: Add ability to use read link in dump_one_reg_file

[Pavel Emelyanov]

On 05/13/2013 07:38 PM, Cyrill Gorcunov wrote:
> 
> To be able to dump procfs namespace entries we will need to
> analyze the link name first and then figure out if the file
> referred indeed a procfs entry or it's plain regular file.
> 
> This means we read link early, and to escape double reading
> of same link, once we read it we remember it in fd_parms
> structure which we pass to dump_one_reg_file.
> 
> Still the dump_one_reg_file is not solely used from inside
> of dump_one_file, but from a number of other places (fifo,
> special files) where fd_parms is filled only partially
> and fill_fd_params is not even called. Thus dump_one_reg_file
> must be ready for such scenario and read the link name by own
> if the caller has not provided one.
> 
> To achieve all this we do
> 
>  - extend fd_parms structure to have a reference on a new
>    fd_link structure, thus if caller already read the link
>    name it might assign the reference and call for dump_one_reg_file
> 
>  - tune up dump_one_reg_file to fill own copy of fd_link
>    structure if the caller has not provied one
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
> ---
>  files-reg.c     | 24 +++++++++++++-----------
>  files.c         | 25 ++++++++++++++++++++++++-
>  include/files.h |  9 +++++++++
>  3 files changed, 46 insertions(+), 12 deletions(-)
> 


> diff --git a/files-reg.c b/files-reg.c
> index 684cfd3..a473fab 100644
> --- a/files-reg.c
> +++ b/files-reg.c
> @@ -432,36 +432,38 @@ static int check_path_remap(char *rpath, int plen, const struct stat *ost, int l
>  
>  int dump_one_reg_file(int lfd, u32 id, const struct fd_parms *p)
>  {
> -	char rpath[PATH_MAX + 1] = ".", *path = rpath + 1;
> -	int len, rfd;
> +	struct fd_link _link, *link;
> +	int rfd;
>  
>  	RegFileEntry rfe = REG_FILE_ENTRY__INIT;
>  
> -	len = read_fd_link(lfd, path, sizeof(rpath) - 1);
> -	if (len < 0) {
> -		pr_err("Can't read link\n");
> -		return len;
> +	if (!p->link) {
> +		if (fill_fdlink(lfd, p, &_link))

Make fill_fdlink assign p->link = &link and thus if would look like

if (!p->link)
	fill_fdlink(lfd, p, &_link);

link = p->link;

and (see below) ...

> +			return -1;
> +		link = &_link;
> +	} else {
> +		link = p->link;
>  	}
>  
>  	pr_info("Dumping path for %d fd via self %d [%s]\n",
> -			p->fd, lfd, path);
> +			p->fd, lfd, &link->name[1]);
>  
>  	/*
>  	 * The regular path we can handle should start with slash.
>  	 */
> -	if (path[0] != '/') {
> -		pr_err("The path [%s] is not supported\n", path);
> +	if (link->name[1] != '/') {
> +		pr_err("The path [%s] is not supported\n", &link->name[1]);
>  		return -1;
>  	}
>  
> -	if (check_path_remap(rpath, len, &p->stat, lfd, id))
> +	if (check_path_remap(link->name, link->len, &p->stat, lfd, id))
>  		return -1;
>  
>  	rfe.id		= id;
>  	rfe.flags	= p->flags;
>  	rfe.pos		= p->pos;
>  	rfe.fown	= (FownEntry *)&p->fown;
> -	rfe.name	= path;
> +	rfe.name	= &link->name[1];
>  
>  	rfd = fdset_fd(glob_fdset, CR_FD_REG_FILES);
>  
> diff --git a/files.c b/files.c
> index 786eca7..24eb34a 100644
> --- a/files.c
> +++ b/files.c
> @@ -135,6 +135,23 @@ int do_dump_gen_file(struct fd_parms *p, int lfd,
>  	return pb_write_one(fdinfo, &e, PB_FDINFO);
>  }
>  
> +int fill_fdlink(int lfd, const struct fd_parms *p, struct fd_link *link)
> +{
> +	char *path = &link->name[1];
> +	size_t len;
> +
> +	link->name[0] = '.';
> +
> +	len = read_fd_link(lfd, path, sizeof(link->name) - 1);
> +	if (len < 0) {
> +		pr_err("Can't read link for pid %d fd %d\n", p->pid, p->fd);
> +		return -1;
> +	}
> +
> +	link->len = len + 1;
> +	return 0;
> +}
> +
>  static int fill_fd_params(struct parasite_ctl *ctl, int fd, int lfd,
>  				struct fd_opts *opts, struct fd_parms *p)
>  {
> @@ -244,8 +261,14 @@ static int dump_one_file(struct parasite_ctl *ctl, int fd, int lfd, struct fd_op
>  			return dump_unsupp_fd(&p);
>  	}
>  
> -	if (S_ISREG(p.stat.st_mode) || S_ISDIR(p.stat.st_mode))
> +	if (S_ISREG(p.stat.st_mode) || S_ISDIR(p.stat.st_mode)) {
> +		struct fd_link link;
> +
> +		if (fill_fdlink(lfd, &p, &link))
> +			return -1;
> +		p.link = &link;

This place would be shorter.

>  		return dump_reg_file(&p, lfd, fdinfo);
> +	}
>  
>  	if (S_ISFIFO(p.stat.st_mode)) {
>  		if (statfs.f_type == PIPEFS_MAGIC)
> diff --git a/include/files.h b/include/files.h
> index 5ecc550..6331911 100644
> --- a/include/files.h
> +++ b/include/files.h
> @@ -17,6 +17,11 @@ struct cr_fdset;
>  struct rst_info;
>  struct parasite_ctl;
>  
> +struct fd_link {
> +	char		name[PATH_MAX + 1];

Is it nice to put 2 more PAGE_SIZE things on stack?

> +	size_t		len;
> +};
> +
>  struct fd_parms {
>  	int		fd;
>  	unsigned long	pos;
> @@ -25,6 +30,7 @@ struct fd_parms {
>  	struct stat	stat;
>  	pid_t		pid;
>  	FownEntry	fown;
> +	struct fd_link	*link;
>  
>  	struct parasite_ctl *ctl;
>  };
> @@ -33,8 +39,11 @@ struct fd_parms {
>  {					\
>  	.fd	= FD_DESC_INVALID,	\
>  	.fown	= FOWN_ENTRY__INIT,	\
> +	.link	= NULL,			\
>  }
>  
> +extern int fill_fdlink(int lfd, const struct fd_parms *p, struct fd_link *link);
> +
>  struct file_desc;
>  
>  struct fdinfo_list_entry {