[CRIU] [PATCH 03/11] files: split collect_fd on allocate_fd and handle_fd

[Pavel Emelyanov]

On 02/09/2017 01:57 AM, Andrew Vagin wrote:
> On Wed, Feb 08, 2017 at 12:34:34PM +0300, Pavel Emelyanov wrote:
>> On 02/02/2017 03:04 AM, Andrei Vagin wrote:
>>> From: Andrew Vagin <avagin at virtuozzo.com>
>>>
>>> We are going to open root directories for each namespace in
>>> the root task, so we need to collect all file descriptors to
>>> be able to find unused file descriptors.
>>
>> OK, but why can't we use service-fd for the root dir?
> 
> Because we need to open all network namespaces (more than one)

OK. Two concerns here.

1. For the same thing in mount ns restore we don't mess with unused fds. Why?
2. For the same thing in external tty restore we introduced fdstore %) Why
   not unify these two?

>>
>>> Signed-off-by: Andrei Vagin <avagin at virtuozzo.com>
>>> ---
>>>  criu/cr-restore.c    | 18 ++++++++++++++++++
>>>  criu/files.c         | 50 +++++++++++++++++++++++++++++++++++++++++---------
>>>  criu/include/files.h |  1 +
>>>  3 files changed, 60 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/criu/cr-restore.c b/criu/cr-restore.c
>>> index 53eb6b4..ba8ee57 100644
>>> --- a/criu/cr-restore.c
>>> +++ b/criu/cr-restore.c
>>> @@ -1323,6 +1323,21 @@ static int create_children_and_session(void)
>>>  	return 0;
>>>  }
>>>  
>>> +static int collect_fd_shared()
>>> +{
>>> +	struct pstree_item *pi;
>>> +
>>> +	for_each_pstree_item(pi) {
>>> +		if (pi->pid->state == TASK_HELPER)
>>> +			continue;
>>> +
>>> +		if  (collect_fd_pid(pi))
>>> +			return -1;
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>> +
>>>  static int restore_task_with_children(void *_arg)
>>>  {
>>>  	struct cr_clone_arg *ca = _arg;
>>> @@ -1410,6 +1425,9 @@ static int restore_task_with_children(void *_arg)
>>>  		pr_info("Calling restore_sid() for init\n");
>>>  		restore_sid();
>>>  
>>> +		if (collect_fd_shared())
>>> +			goto err;
>>> +
>>>  		/*
>>>  		 * We need non /proc proc mount for restoring pid and mount
>>>  		 * namespaces and do not care for the rest of the cases.
>>> diff --git a/criu/files.c b/criu/files.c
>>> index 1a76313..4ef75b3 100644
>>> --- a/criu/files.c
>>> +++ b/criu/files.c
>>> @@ -686,19 +686,28 @@ int rst_file_params(int fd, FownEntry *fown, int flags)
>>>  	return 0;
>>>  }
>>>  
>>> -static int collect_fd(int pid, FdinfoEntry *e, struct rst_info *rst_info)
>>> +static struct fdinfo_list_entry *allocate_fd(int pid, FdinfoEntry *e, struct rst_info *rst_info)
>>>  {
>>> -	struct fdinfo_list_entry *le, *new_le;
>>> -	struct file_desc *fdesc;
>>> +	struct fdinfo_list_entry *new_le;
>>>  
>>>  	pr_info("Collect fdinfo pid=%d fd=%d id=%#x\n",
>>>  		pid, e->fd, e->id);
>>>  
>>>  	new_le = shmalloc(sizeof(*new_le));
>>>  	if (!new_le)
>>> -		return -1;
>>> +		return NULL;
>>>  
>>>  	fle_init(new_le, pid, e);
>>> +	collect_task_fd(new_le, rst_info);
>>> +
>>> +	return new_le;
>>> +}
>>> +
>>> +static int handle_fd(struct fdinfo_list_entry *new_le, struct rst_info *rst_info)
>>> +{
>>> +	struct fdinfo_list_entry *le;
>>> +	struct file_desc *fdesc;
>>> +	FdinfoEntry *e = new_le->fe;
>>>  
>>>  	fdesc = find_file_desc(e);
>>>  	if (fdesc == NULL) {
>>> @@ -713,14 +722,23 @@ static int collect_fd(int pid, FdinfoEntry *e, struct rst_info *rst_info)
>>>  	if (fdesc->ops->collect_fd)
>>>  		fdesc->ops->collect_fd(fdesc, new_le, rst_info);
>>>  
>>> -	collect_task_fd(new_le, rst_info);
>>> -
>>>  	list_add_tail(&new_le->desc_list, &le->desc_list);
>>>  	new_le->desc = fdesc;
>>>  
>>>  	return 0;
>>>  }
>>>  
>>> +static int collect_fd(int pid, FdinfoEntry *e, struct rst_info *rst_info)
>>> +{
>>> +	struct fdinfo_list_entry *le;
>>> +
>>> +	le = allocate_fd(pid, e, rst_info);
>>> +	if (le == NULL)
>>> +		return -1;
>>> +
>>> +	return handle_fd(le, rst_info);
>>> +}
>>> +
>>>  FdinfoEntry *dup_fdinfo(FdinfoEntry *old, int fd, unsigned flags)
>>>  {
>>>  	FdinfoEntry *e;
>>> @@ -777,7 +795,7 @@ int prepare_ctl_tty(int pid, struct rst_info *rst_info, u32 ctl_tty_id)
>>>  	return 0;
>>>  }
>>>  
>>> -int prepare_fd_pid(struct pstree_item *item)
>>> +int collect_fd_pid(struct pstree_item *item)
>>>  {
>>>  	int ret = 0;
>>>  	struct cr_img *img;
>>> @@ -798,6 +816,7 @@ int prepare_fd_pid(struct pstree_item *item)
>>>  		return -1;
>>>  
>>>  	while (1) {
>>> +		struct fdinfo_list_entry *le;
>>>  		FdinfoEntry *e;
>>>  
>>>  		ret = pb_read_one_eof(img, &e, PB_FDINFO);
>>> @@ -810,9 +829,10 @@ int prepare_fd_pid(struct pstree_item *item)
>>>  			break;
>>>  		}
>>>  
>>> -		ret = collect_fd(pid, e, rst_info);
>>> -		if (ret < 0) {
>>> +		le = allocate_fd(pid, e, rst_info);
>>> +		if (le == NULL) {
>>>  			fdinfo_entry__free_unpacked(e, NULL);
>>> +			ret = -1;
>>>  			break;
>>>  		}
>>>  	}
>>> @@ -821,6 +841,18 @@ int prepare_fd_pid(struct pstree_item *item)
>>>  	return ret;
>>>  }
>>>  
>>> +int prepare_fd_pid(struct pstree_item *item)
>>> +{
>>> +	struct rst_info *rst_info = rsti(item);
>>> +	struct fdinfo_list_entry *le, *t;
>>> +	list_for_each_entry_safe(le, t, &rst_info->fds, ps_list) {
>>> +		if (handle_fd(le, rst_info))
>>> +			return -1;
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>> +
>>>  #define SETFL_MASK (O_APPEND | O_ASYNC | O_NONBLOCK | O_NDELAY | O_DIRECT | O_NOATIME)
>>>  int set_fd_flags(int fd, int flags)
>>>  {
>>> diff --git a/criu/include/files.h b/criu/include/files.h
>>> index 39ef23b..f1216a1 100644
>>> --- a/criu/include/files.h
>>> +++ b/criu/include/files.h
>>> @@ -156,6 +156,7 @@ extern int rst_file_params(int fd, FownEntry *fown, int flags);
>>>  extern void show_saved_files(void);
>>>  
>>>  extern int prepare_fds(struct pstree_item *me);
>>> +extern int collect_fd_pid(struct pstree_item *me);
>>>  extern int prepare_fd_pid(struct pstree_item *me);
>>>  extern int prepare_ctl_tty(int pid, struct rst_info *rst_info, u32 ctl_tty_id);
>>>  extern int prepare_shared_fdinfo(void);
>>>
>>
> .
>