[CRIU] [PATCH v2 07/28] files: Allow to receive further fds

[Kirill Tkhai]

On 05.12.2016 12:57, Pavel Emelyanov wrote:
> On 11/30/2016 07:30 PM, Kirill Tkhai wrote:
>> For moving to a single transport socket scheme, we should be able
>> to receive a fd, which is not need at the moment, but it will
>> be used in the future. So, we receive a further fd, and then
>> continue to wait the fd, we really need now.
>>
>> Signed-off-by: Kirill Tkhai <ktkhai at virtuozzo.com>
>> ---
>>  criu/files.c         |   37 +++++++++++++++++++++++++++++++++++--
>>  criu/include/files.h |    2 ++
>>  2 files changed, 37 insertions(+), 2 deletions(-)
>>
>> diff --git a/criu/files.c b/criu/files.c
>> index de4ef17..806af26 100644
>> --- a/criu/files.c
>> +++ b/criu/files.c
>> @@ -956,20 +956,51 @@ static int open_transport_fd(int pid, struct fdinfo_list_entry *fle)
>>  	return -1;
>>  }
>>  
>> +static bool insane_fle(struct fdinfo_list_entry *alien_fle)
> 
> So is it insane or alien? :)
> 
>> +{
>> +	struct fdinfo_list_entry *fle;
>> +
>> +	list_for_each_entry(fle, &rsti(current)->used, used_list)
>> +		if (alien_fle == fle)
>> +			return false;
>> +	pr_err("Insane fle=%p, pid=%d\n", alien_fle, current->pid.virt);
>> +	return true;
>> +}
>> +
>> +static int recv_further_fle(struct fdinfo_list_entry *fle, int fd)
> 
> This routine keeps alien (or insane?) fle in local task for future use. Let's
> better call this one 'park_fd' or 'keep_fd_for_future'. Some name to reflect
> the fact that fd is not received, but is kept until better times.

OK
 
>> +{
>> +	BUG_ON(fle->received);
>> +	fle->received = 1;
>> +	if (close(fle->fe->fd) < 0) {
>> +		pr_perror("Can't close transport fd\n");
>> +		return -1;
>> +	}
>> +	return reopen_fd_as(fle->fe->fd, fd);
>> +}
>> +
>>  int recv_fd_from_peer(struct fdinfo_list_entry *fle)
>>  {
>>  	struct fdinfo_list_entry *tmp;
>>  	int fd, ret;
>>  
>> +	if (fle->received)
>> +		return fle->fe->fd;
>> +again:
>>  	ret = recv_fds(fle->fe->fd, &fd, 1, (void *)&tmp, sizeof(struct fdinfo_list_entry *));
>>  	if (ret)
>>  		return -1;
>>  
>>  	if (tmp != fle) {
>> -		pr_err("Received wrong fle\n");
>> -		return -1;
>> +		pr_info("Further fle=%p, pid=%d\n", tmp, fle->pid);
>> +		if (insane_fle(fle))
>> +			return -1;
>> +		if (recv_further_fle(tmp, fd))
>> +			return -1;
>> +		goto again;
>>  	}
>>  	close(fle->fe->fd);
>> +	BUG_ON(fle->received);
> 
> if (fle->received) is checked above, why do we need BUG_ON() here?

Sure, it will be removed in v3!

>> +	fle->received = 1;
>>  
>>  	return fd;
>>  }
>> @@ -1013,6 +1044,8 @@ static int send_fd_to_self(int fd, struct fdinfo_list_entry *fle)
>>  		return -1;
>>  	}
>>  
>> +	fle->received = 1;
>> +
>>  	return 0;
>>  }
>>  
>> diff --git a/criu/include/files.h b/criu/include/files.h
>> index c9b59c3..bd20e59 100644
>> --- a/criu/include/files.h
>> +++ b/criu/include/files.h
>> @@ -72,12 +72,14 @@ struct fdinfo_list_entry {
>>  	int			pid;
>>  	futex_t			real_pid;
>>  	FdinfoEntry		*fe;
>> +	u8			received:1;
> 
> bool?

Bool is compiler-dependent, and often it is int. In kernel we use int:1 instead
 
>>  };
>>  
>>  static inline void fle_init(struct fdinfo_list_entry *fle, int pid, FdinfoEntry *fe)
>>  {
>>  	fle->pid = pid;
>>  	fle->fe = fe;
>> +	fle->received = 0;
>>  }
>>  
>>  /* reports whether fd_a takes prio over fd_b */
>>
>> .
>>
>