[CRIU] Re: [PATCH v2] crtools: cleanup fdset initalization

Pavel Emelyanov xemul at parallels.com
Wed Feb 8 12:38:03 EST 2012


On 02/08/2012 09:19 PM, Kinsbursky Stanislav wrote:
> v2: wrappers names become less obfuscating
> 
> This patch:
> 1) Updates function cr_fdset_open() to be suitable for handling fdset creation
> for dump and show stages.
> 2) Replaces cr_fdset_open() by new wrapper function cr_fdset_dump().
> 3) Replaces prep_cr_fdset_for_restore() by new wrapper function cr_fdset_show().
> 
> Signed-off-by: Stanislav Kinsbursky <skinsbursky at parallels.com>

Acked-by: Pavel Emelyanov <xemul at parallels.com>

> ---
>  cr-dump.c         |   10 +++---
>  cr-show.c         |    6 ++--
>  crtools.c         |   82 +++++++++++++++++++++--------------------------------
>  include/crtools.h |    4 +--
>  namespaces.c      |    4 +--
>  5 files changed, 45 insertions(+), 61 deletions(-)
> 
> diff --git a/cr-dump.c b/cr-dump.c
> index ce101b1..97c0ca6 100644
> --- a/cr-dump.c
> +++ b/cr-dump.c
> @@ -1187,7 +1187,7 @@ static int dump_one_zombie(struct pstree_item *item, struct proc_pid_stat *pps,
>  {
>  	struct core_entry *core;
>  
> -	cr_fdset = cr_fdset_open(item->pid, CR_FD_DESC_CORE, cr_fdset);
> +	cr_fdset = cr_dump_fdset_open(item->pid, CR_FD_DESC_CORE, cr_fdset);
>  	if (cr_fdset == NULL)
>  		return -1;
>  
> @@ -1216,7 +1216,7 @@ static int dump_task_threads(struct pstree_item *item)
>  		if (item->pid == item->threads[i])
>  			continue;
>  
> -		cr_fdset_thread = cr_fdset_open(item->threads[i], CR_FD_DESC_CORE, NULL);
> +		cr_fdset_thread = cr_dump_fdset_open(item->threads[i], CR_FD_DESC_CORE, NULL);
>  		if (!cr_fdset_thread)
>  			goto err;
>  
> @@ -1266,7 +1266,7 @@ static int dump_one_task(struct pstree_item *item, struct cr_fdset *cr_fdset)
>  		return dump_one_zombie(item, &pps_buf, cr_fdset);
>  
>  	ret = -1;
> -	if (!cr_fdset_open(item->pid, CR_FD_DESC_TASK, cr_fdset))
> +	if (!cr_dump_fdset_open(item->pid, CR_FD_DESC_TASK, cr_fdset))
>  		goto err;
>  
>  	ret = collect_mappings(pid, pid_dir, &vma_area_list);
> @@ -1383,12 +1383,12 @@ int cr_dump_tasks(pid_t pid, struct cr_options *opts)
>  	collect_sockets();
>  
>  	list_for_each_entry(item, &pstree_list, list) {
> -		cr_fdset = cr_fdset_open(item->pid, CR_FD_DESC_NONE, NULL);
> +		cr_fdset = cr_dump_fdset_open(item->pid, CR_FD_DESC_NONE, NULL);
>  		if (!cr_fdset)
>  			goto err;
>  
>  		if (item->pid == pid) {
> -			if (!cr_fdset_open(item->pid, CR_FD_DESC_USE(CR_FD_PSTREE), cr_fdset))
> +			if (!cr_dump_fdset_open(item->pid, CR_FD_DESC_USE(CR_FD_PSTREE), cr_fdset))
>  				goto err;
>  			if (dump_pstree(pid, &pstree_list, cr_fdset))
>  				goto err;
> diff --git a/cr-show.c b/cr-show.c
> index 33e60b8..948f51f 100644
> --- a/cr-show.c
> +++ b/cr-show.c
> @@ -535,7 +535,7 @@ static int cr_show_all(unsigned long pid, struct cr_options *opts)
>  	LIST_HEAD(pstree_list);
>  	int i, ret = -1;
>  
> -	cr_fdset = prep_cr_fdset_for_restore(pid, CR_FD_DESC_PSTREE);
> +	cr_fdset = cr_show_fdset_open(pid, CR_FD_DESC_PSTREE);
>  	if (!cr_fdset)
>  		goto out;
>  
> @@ -551,7 +551,7 @@ static int cr_show_all(unsigned long pid, struct cr_options *opts)
>  
>  	list_for_each_entry(item, &pstree_list, list) {
>  
> -		cr_fdset = prep_cr_fdset_for_restore(item->pid, CR_FD_DESC_TASK);
> +		cr_fdset = cr_show_fdset_open(item->pid, CR_FD_DESC_TASK);
>  		if (!cr_fdset)
>  			goto out;
>  
> @@ -566,7 +566,7 @@ static int cr_show_all(unsigned long pid, struct cr_options *opts)
>  				if (item->threads[i] == item->pid)
>  					continue;
>  
> -				cr_fdset_th = prep_cr_fdset_for_restore(item->threads[i], CR_FD_DESC_CORE);
> +				cr_fdset_th = cr_show_fdset_open(item->threads[i], CR_FD_DESC_CORE);
>  				if (!cr_fdset_th)
>  					goto out;
>  
> diff --git a/crtools.c b/crtools.c
> index 8cfe866..6f6c254 100644
> --- a/crtools.c
> +++ b/crtools.c
> @@ -157,7 +157,8 @@ void close_cr_fdset(struct cr_fdset **cr_fdset)
>  	*cr_fdset = NULL;
>  }
>  
> -struct cr_fdset *cr_fdset_open(int pid, unsigned long use_mask, struct cr_fdset *cr_fdset)
> +static struct cr_fdset *cr_fdset_open(int pid, unsigned long use_mask,
> +			       unsigned long flags, struct cr_fdset *cr_fdset)
>  {
>  	struct cr_fdset *fdset;
>  	unsigned int i;
> @@ -186,22 +187,38 @@ struct cr_fdset *cr_fdset_open(int pid, unsigned long use_mask, struct cr_fdset
>  		if (ret)
>  			goto err;
>  
> -		ret = unlink(path);
> -		if (ret && errno != ENOENT) {
> -			pr_perror("Unable to unlink %s", path);
> -			goto err;
> +		if (flags & O_EXCL) {
> +			ret = unlink(path);
> +			if (ret && errno != ENOENT) {
> +				pr_perror("Unable to unlink %s", path);
> +				goto err;
> +			}
>  		}
>  
> -		ret = open(path, O_RDWR | O_CREAT | O_EXCL, CR_FD_PERM);
> +		ret = open(path, flags, CR_FD_PERM);
>  		if (ret < 0) {
> +			if (!(flags & O_CREAT))
> +				/* caller should check himself */
> +				continue;
>  			pr_perror("Unable to open %s", path);
>  			goto err;
>  		}
>  		fdset->fds[i] = ret;
>  
>  		pr_debug("Opened %s with %d\n", path, ret);
> -		if (write_img(ret, &fdset_template[i].magic))
> -			goto err;
> +		if (flags == O_RDONLY) {
> +			u32 magic;
> +
> +			if (read_img(ret, &magic) < 0)
> +				goto err;
> +			if (magic != fdset_template[i].magic) {
> +				pr_err("Magic doesn't match for %s\n", path);
> +				goto err;
> +			}
> +		} else {
> +			if (write_img(ret, &fdset_template[i].magic))
> +				goto err;
> +		}
>  	}
>  
>  	return fdset;
> @@ -214,49 +231,16 @@ err:
>  	return NULL;
>  }
>  
> -struct cr_fdset *prep_cr_fdset_for_restore(int pid, unsigned long use_mask)
> +struct cr_fdset *cr_dump_fdset_open(int pid, unsigned long use_mask,
> +				     struct cr_fdset *cr_fdset)
>  {
> -	unsigned int i;
> -	int ret = -1;
> -	char path[PATH_MAX];
> -	u32 magic;
> -	struct cr_fdset *cr_fdset;
> -
> -	cr_fdset = alloc_cr_fdset();
> -	if (!cr_fdset)
> -		goto err;
> -
> -	for (i = 0; i < CR_FD_MAX; i++) {
> -		if (!(use_mask & CR_FD_DESC_USE(i)))
> -			continue;
> -
> -		ret = get_image_path(path, sizeof(path),
> -				fdset_template[i].fmt, pid);
> -		if (ret)
> -			goto err;
> -
> -		ret = open(path, O_RDWR, CR_FD_PERM);
> -		if (ret < 0) {
> -			pr_perror("Unable to open %s", path);
> -			goto err;
> -		}
> -		cr_fdset->fds[i] = ret;
> -
> -		pr_debug("Opened %s with %d\n", path, ret);
> -		if (read_img(ret, &magic) < 0)
> -			goto err;
> -		if (magic != fdset_template[i].magic) {
> -			pr_err("Magic doesn't match for %s\n", path);
> -			goto err;
> -		}
> -
> -	}
> -
> -	return cr_fdset;
> +	return cr_fdset_open(pid, use_mask, O_RDWR | O_CREAT | O_EXCL,
> +			     cr_fdset);
> +}
>  
> -err:
> -	close_cr_fdset(&cr_fdset);
> -	return NULL;
> +struct cr_fdset *cr_show_fdset_open(int pid, unsigned long use_mask)
> +{
> +	return cr_fdset_open(pid, use_mask, O_RDONLY, NULL);
>  }
>  
>  static int parse_ns_string(const char *ptr, unsigned int *flags)
> diff --git a/include/crtools.h b/include/crtools.h
> index acb6a6d..868ab38 100644
> --- a/include/crtools.h
> +++ b/include/crtools.h
> @@ -119,8 +119,8 @@ int cr_restore_tasks(pid_t pid, struct cr_options *opts);
>  int cr_show(unsigned long pid, struct cr_options *opts);
>  int convert_to_elf(char *elf_path, int fd_core);
>  
> -struct cr_fdset *cr_fdset_open(int pid, unsigned long use_mask, struct cr_fdset *);
> -struct cr_fdset *prep_cr_fdset_for_restore(int pid, unsigned long use_mask);
> +struct cr_fdset *cr_dump_fdset_open(int pid, unsigned long use_mask, struct cr_fdset *);
> +struct cr_fdset *cr_show_fdset_open(int pid, unsigned long use_mask);
>  void close_cr_fdset(struct cr_fdset **cr_fdset);
>  
>  void free_mappings(struct list_head *vma_area_list);
> diff --git a/namespaces.c b/namespaces.c
> index 1e245b8..75ae142 100644
> --- a/namespaces.c
> +++ b/namespaces.c
> @@ -33,7 +33,7 @@ static int do_dump_namespaces(int ns_pid, unsigned int ns_flags)
>  	struct cr_fdset *fdset;
>  	int ret = 0;
>  
> -	fdset = cr_fdset_open(ns_pid, CR_FD_DESC_NS, NULL);
> +	fdset = cr_dump_fdset_open(ns_pid, CR_FD_DESC_NS, NULL);
>  	if (fdset == NULL)
>  		return -1;
>  
> @@ -118,7 +118,7 @@ int try_show_namespaces(int ns_pid)
>  {
>  	struct cr_fdset *fdset;
>  
> -	fdset = prep_cr_fdset_for_restore(ns_pid, CR_FD_DESC_NS);
> +	fdset = cr_show_fdset_open(ns_pid, CR_FD_DESC_NS);
>  	if (!fdset)
>  		return -1;
>  
> 



More information about the CRIU mailing list