[CRIU] [PATCH v2 03/28] files: Send/receive fds with pointers on their struct fdinfo_list_entry in receiver

[Pavel Emelyanov]

On 12/05/2016 04:05 PM, Kirill Tkhai wrote:
> On 05.12.2016 15:50, Pavel Emelyanov wrote:
>> On 12/05/2016 01:55 PM, Kirill Tkhai wrote:
>>> On 05.12.2016 12:50, Pavel Emelyanov wrote:
>>>> On 11/30/2016 07:29 PM, Kirill Tkhai wrote:
>>>>> For moving to a single transport socket scheme, we need to be able to differ
>>>>> fds in receive queue from each other. Add a fle pointer as identifier for that.
>>>>>
>>>>> v2: Rebase on compel
>>>>> Signed-off-by: Kirill Tkhai <ktkhai at virtuozzo.com>
>>>>> ---
>>>>>  criu/files.c         |   21 +++++++++++++++++++--
>>>>>  criu/include/files.h |    1 +
>>>>>  criu/pipes.c         |    2 +-
>>>>>  criu/sk-unix.c       |    2 +-
>>>>>  criu/tty.c           |    2 +-
>>>>>  5 files changed, 23 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/criu/files.c b/criu/files.c
>>>>> index e691558..c3ea2c4 100644
>>>>> --- a/criu/files.c
>>>>> +++ b/criu/files.c
>>>>> @@ -957,6 +957,23 @@ static int open_transport_fd(int pid, struct fdinfo_list_entry *fle)
>>>>>  	return -1;
>>>>>  }
>>>>>  
>>>>> +int recv_fd_from_peer(struct fdinfo_list_entry *fle)
>>>>> +{
>>>>> +	struct fdinfo_list_entry *tmp;
>>>>> +	int fd, ret;
>>>>> +
>>>>> +	ret = recv_fds(fle->fe->fd, &fd, 1, (void *)&tmp, sizeof(struct fdinfo_list_entry *));
>>>>> +	if (ret)
>>>>> +		return -1;
>>>>> +
>>>>> +	if (tmp != fle) {
>>>>
>>>> Can this patch be applied by its own? I mean each pstree-item allocates its
>>>> own fle-s, their address cannot match to each other.
>>>
>>> Yes, it can be applied by its own. Each fle is related to certain process,
>>> which is specified in fle->pid.
>>
>> So the check for tmp != fle will always be true, will it?
> 
> It has to be always false, as we receive the only fd from per-fd transport socket.

But how will it be false? You call send_fds() passing a pointer to the sender's
fle structure as an argument, then in another task go to this routine and receive
this pointer, then compare it the this other task's local pointer. How can
a != b be false in this scenario?

-- Pavel

>>>>> +		pr_err("Received wrong fle\n");
>>>>> +		return -1;
>>>>> +	}
>>>>> +
>>>>> +	return fd;
>>>>> +}
>>>>> +
>>>>>  int send_fd_to_peer(int fd, struct fdinfo_list_entry *fle, int sock)
>>>>>  {
>>>>>  	struct sockaddr_un saddr;
>>>>> @@ -967,7 +984,7 @@ int send_fd_to_peer(int fd, struct fdinfo_list_entry *fle, int sock)
>>>>>  	transport_name_gen(&saddr, &len,
>>>>>  			futex_get(&fle->real_pid), fle->fe->fd);
>>>>>  	pr_info("\t\tSend fd %d to %s\n", fd, saddr.sun_path + 1);
>>>>> -	return send_fd(sock, &saddr, len, fd);
>>>>> +	return send_fds(sock, &saddr, len, &fd, 1, (void *)&fle, sizeof(struct fdinfo_list_entry *));
>>>>>  }
>>>>>  
>>>>>  static int send_fd_to_self(int fd, struct fdinfo_list_entry *fle, int sock)
>>>>> @@ -1077,7 +1094,7 @@ static int receive_fd(int pid, struct fdinfo_list_entry *fle)
>>>>>  
>>>>>  	pr_info("\tReceive fd for %d\n", fle->fe->fd);
>>>>>  
>>>>> -	tmp = recv_fd(fle->fe->fd);
>>>>> +	tmp = recv_fd_from_peer(fle);
>>>>>  	if (tmp < 0) {
>>>>>  		pr_err("Can't get fd %d\n", tmp);
>>>>>  		return -1;
>>>>> diff --git a/criu/include/files.h b/criu/include/files.h
>>>>> index 229c0d2..4f40e71 100644
>>>>> --- a/criu/include/files.h
>>>>> +++ b/criu/include/files.h
>>>>> @@ -148,6 +148,7 @@ extern int file_desc_add(struct file_desc *d, u32 id, struct file_desc_ops *ops)
>>>>>  extern struct fdinfo_list_entry *file_master(struct file_desc *d);
>>>>>  extern struct file_desc *find_file_desc_raw(int type, u32 id);
>>>>>  
>>>>> +extern int recv_fd_from_peer(struct fdinfo_list_entry *fle);
>>>>>  extern int send_fd_to_peer(int fd, struct fdinfo_list_entry *fle, int sock);
>>>>>  extern int restore_fown(int fd, FownEntry *fown);
>>>>>  extern int rst_file_params(int fd, FownEntry *fown, int flags);
>>>>> diff --git a/criu/pipes.c b/criu/pipes.c
>>>>> index 9fc87bd..31f436e 100644
>>>>> --- a/criu/pipes.c
>>>>> +++ b/criu/pipes.c
>>>>> @@ -237,7 +237,7 @@ static int recv_pipe_fd(struct pipe_info *pi)
>>>>>  
>>>>>  	pr_info("\tWaiting fd for %d\n", fd);
>>>>>  
>>>>> -	tmp = recv_fd(fd);
>>>>> +	tmp = recv_fd_from_peer(fle);
>>>>>  	if (tmp < 0) {
>>>>>  		pr_err("Can't get fd %d\n", tmp);
>>>>>  		return -1;
>>>>> diff --git a/criu/sk-unix.c b/criu/sk-unix.c
>>>>> index 202228d..74bd76d 100644
>>>>> --- a/criu/sk-unix.c
>>>>> +++ b/criu/sk-unix.c
>>>>> @@ -1096,7 +1096,7 @@ static int open_unixsk_pair_slave(struct unix_sk_info *ui)
>>>>>  	pr_info("Opening pair slave (id %#x ino %#x peer %#x) on %d\n",
>>>>>  			ui->ue->id, ui->ue->ino, ui->ue->peer, fle->fe->fd);
>>>>>  
>>>>> -	sk = recv_fd(fle->fe->fd);
>>>>> +	sk = recv_fd_from_peer(fle);
>>>>>  	if (sk < 0) {
>>>>>  		pr_err("Can't recv pair slave\n");
>>>>>  		return -1;
>>>>> diff --git a/criu/tty.c b/criu/tty.c
>>>>> index 7822a4f..6d0c8fa 100644
>>>>> --- a/criu/tty.c
>>>>> +++ b/criu/tty.c
>>>>> @@ -923,7 +923,7 @@ static int receive_tty(struct tty_info *info)
>>>>>  	fle = file_master(&info->d);
>>>>>  	pr_info("\tWaiting tty fd %d (pid %d)\n", fle->fe->fd, fle->pid);
>>>>>  
>>>>> -	fd = recv_fd(fle->fe->fd);
>>>>> +	fd = recv_fd_from_peer(fle);
>>>>>  	close(fle->fe->fd);
>>>>>  	if (fd < 0) {
>>>>>  		pr_err("Can't get fd %d\n", fd);
>>>>>
>>>>> .
>>>>>
>>>>
>>> .
>>>
>>
> .
>