[CRIU] Re: [PATCH 2/4] sockets: Restore unconnected dgram sockets

[Pavel Emelyanov]

> @@ -478,6 +481,7 @@ usage:
>  	pr_msg("  -s             leave tasks in stopped state after checkpoint instead of killing them\n");
>  	pr_msg("  -n             checkpoint/restore namespaces - values must be separated by comma\n");
>  	pr_msg("                 supported: uts, ipc\n");
> +	pr_msg("  -r             resolve unconnected socket peer connection by name\n");

I'd make the option meaning "allow external unix connections".

>  
>  	pr_msg("\nAdditional common parameters:\n");
>  	pr_msg("  -D dir         specifis directory where checkpoint files are/to be located\n");
> diff --git a/include/crtools.h b/include/crtools.h
> index d79878f..764cfe1 100644
> --- a/include/crtools.h
> +++ b/include/crtools.h
> @@ -67,6 +67,7 @@ struct cr_options {
>  	bool			leader_only;
>  	bool			show_pages_content;
>  	bool			restore_detach;
> +	bool			resolve_peer_by_name;
>  	unsigned int		namespaces_flags;
>  };
>  
> @@ -160,6 +161,7 @@ static inline int fdset_fd(const struct cr_fdset *fdset, int type)
>  }
>  
>  extern struct cr_fdset *glob_fdset;
> +extern struct cr_options opts;
>  
>  int cr_dump_tasks(pid_t pid, const struct cr_options *opts);
>  int cr_restore_tasks(pid_t pid, struct cr_options *opts);
> diff --git a/sockets.c b/sockets.c
> index 4428eae..09756e9 100644
> --- a/sockets.c
> +++ b/sockets.c
> @@ -38,6 +38,8 @@ static char buf[4096];
>  #define SOCKFS_MAGIC	0x534F434B
>  #endif
>  
> +#define SOCK_STATE_INFO	255
> +
>  struct socket_desc {
>  	unsigned int		family;
>  	unsigned int		ino;
> @@ -412,10 +414,32 @@ static int dump_one_unix(const struct socket_desc *_sk, struct fd_parms *p,
>  		 * Peer should have us as peer or have a name by which
>  		 * we can access one.
>  		 */
> -		if (!peer->name && (peer->peer_ino != ue.id)) {
> -			pr_err("Unix socket %x with unreachable peer %x (%x/%s)\n",
> -					ue.id, ue.peer, peer->peer_ino, peer->name);
> -			goto err;
> +		if (peer->peer_ino != ue.id) {
> +			if (!peer->name) {
> +				pr_err("Unix socket %x with unreachable peer %x (%x/%s)\n",
> +				       ue.id, ue.peer, peer->peer_ino, peer->name);
> +				goto err;
> +			}
> +
> +			struct unix_sk_entry pt = { };
> +
> +			/*
> +			 * Socket is not connected back but have a name,
> +			 * so at restore time we will find it here. Note
> +			 * the id is set to impossible value by purpose.
> +			 */
> +
> +			pt.id		= -1u;

This is wrong. You can have two or more sockets in the "image" to be connected
to this "external" one. E.g. two daemons connected to /dev/log

> +			pt.peer		= ue.id;
> +			pt.type		= SOCK_DGRAM;
> +			pt.state	= SOCK_STATE_INFO;
> +			pt.namelen	= peer->namelen;
> +
> +			if (write_img(fdset_fd(glob_fdset, CR_FD_UNIXSK), &pt))
> +				goto err;
> +			if (write_img_buf(fdset_fd(glob_fdset, CR_FD_UNIXSK),
> +					  peer->name, pt.namelen))
> +				goto err;
>  		}
>  	} else if (ue.state == TCP_ESTABLISHED) {
>  		const struct unix_sk_listen_icon *e;
> @@ -814,6 +838,7 @@ struct unix_sk_info {
>  #define USK_PAIR_SLAVE		0x2
>  
>  static LIST_HEAD(unix_sockets);
> +static LIST_HEAD(unix_sockets_peer_names);

I don't see much profit in having separate list for external sockets.
The existing infrastructure just allows for adding them in the common
trunk.

>  
>  static struct unix_sk_info *find_unix_sk(int id)
>  {
> @@ -825,6 +850,18 @@ static struct unix_sk_info *find_unix_sk(int id)
>  	return NULL;
>  }
>  
> +static struct unix_sk_info *find_peer_name_sk(int id)
> +{
> +	struct unix_sk_info *info;
> +
> +	list_for_each_entry(info, &unix_sockets_peer_names, list) {
> +		if (info->ue.id == id)
> +			return info;
> +	}
> +
> +	return NULL;
> +}
> +
>  struct sk_packet {
>  	struct list_head list;
>  	struct sk_packet_entry entry;
> @@ -1076,6 +1113,8 @@ static inline char *skstate2s(u32 state)
>  		return "closed";
>  	else if (state == TCP_LISTEN)
>  		return "listen";
> +	else if (state == SOCK_STATE_INFO)
> +		return "  info";

As described above -- external.

>  	else
>  		return unknown(state);
>  }
> @@ -1461,6 +1500,13 @@ int collect_unix_sockets(void)
>  
>  		ui->peer = NULL;
>  		ui->flags = 0;
> +
> +		if (ui->ue.state == SOCK_STATE_INFO) {
> +			pr_info(" `- Got peer name for %u\n", ui->ue.id);
> +			list_add(&ui->list, &unix_sockets_peer_names);
> +			continue;
> +		}
> +
>  		pr_info(" `- Got %u peer %u\n", ui->ue.id, ui->ue.peer);
>  		file_desc_add(&ui->d, FDINFO_UNIXSK, ui->ue.id,
>  				&unix_desc_ops);
> @@ -1485,9 +1531,13 @@ int resolve_unix_peers(void)
>  
>  		peer = find_unix_sk(ui->ue.peer);
>  		if (!peer) {
> -			pr_err("FATAL: Peer %x unresolved for %x\n",
> -					ui->ue.peer, ui->ue.id);
> -			return -1;
> +			if (opts.resolve_peer_by_name)
> +				peer = find_peer_name_sk(ui->ue.peer);
> +			if (!peer) {
> +				pr_err("FATAL: Peer %x unresolved for %x\n",
> +				       ui->ue.peer, ui->ue.id);
> +				return -1;
> +			}
>  		}
>  
>  		ui->peer = peer;