[CRIU] Re: [PATCH 8/8] sockets: Restore in-flight unix stream sockets

[Pavel Emelyanov]

On 01/27/2012 09:38 PM, Cyrill Gorcunov wrote:
> On Fri, Jan 27, 2012 at 08:15:15PM +0400, Cyrill Gorcunov wrote:
>  
>> я могу имя поменять на usk_listeners, но сути это сильно не изменит.
>> Подумаю...
>>
> 
> Something like below I guess? Two hashes -- one for sockets at dump
> time and one to use at restore time as sockets names keeper.

Close to perfect! Some comments inside.

> 	Cyrill
> ---
> diff --git a/sockets.c b/sockets.c
> index 7689b37..684587a 100644
> --- a/sockets.c
> +++ b/sockets.c
> @@ -55,6 +55,19 @@ struct unix_sk_desc {
>  	unsigned int		*icons;
>  };
>  
> +struct unix_sk_listen_icon {
> +	unsigned int			ino;

peer_ino.

> +	struct unix_sk_desc		*sk_desc;
> +	struct unix_sk_listen_icon	*next;
> +};
> +
> +struct unix_sk_listen {
> +	unsigned int			ino;
> +	struct sockaddr_un		addr;
> +	unsigned int			addrlen;
> +	struct unix_sk_listen		*next;
> +};
> +
>  #define INET_ADDR_LEN		40
>  
>  struct inet_sk_desc {
> @@ -75,6 +88,7 @@ struct inet_sk_desc {
>  		(elem)->next = (head)[(key) % SK_HASH_SIZE];		\
>  		(head)[(key) % SK_HASH_SIZE] = (elem);			\
>  	} while (0)
> +

Garbage.

>  #define __gen_static_lookup_func(ret, name, head, _member, _type, _name)\
>  	static ret *name(_type _name) {					\
>  		ret *d;							\
> @@ -88,6 +102,18 @@ struct inet_sk_desc {
>  static struct socket_desc *sockets[SK_HASH_SIZE];
>  __gen_static_lookup_func(struct socket_desc, lookup_socket, sockets, ino, int, ino);
>  
> +static struct unix_sk_listen_icon *unix_listen_icons[SK_HASH_SIZE];
> +__gen_static_lookup_func(struct unix_sk_listen_icon,			\
> +			 lookup_unix_listen_icons,			\
> +			 unix_listen_icons,				\
> +			 ino, unsigned int, ino);
> +
> +static struct unix_sk_listen *unix_listen[SK_HASH_SIZE];
> +__gen_static_lookup_func(struct unix_sk_listen,				\
> +			 lookup_unix_listen,				\
> +			 unix_listen,					\
> +			 ino, unsigned int, ino);
> +
>  static int sk_collect_one(int ino, int family, struct socket_desc *d)
>  {
>  	d->ino		= ino;
> @@ -262,9 +288,35 @@ static int dump_one_unix(struct socket_desc *_sk, int fd, struct cr_fdset *cr_fd
>  	ue.namelen	= sk->namelen;
>  	ue.backlog	= sk->wqlen;
>  
> -	ue.pad		= 0;
> +	ue.flags	= USK_REGULAR;

ue.flags == 0;

Only one flag is required called USK_INFLIGHT;

>  	ue.peer		= sk->peer_ino;
>  
> +	/*
> +	 * If this is in-flight connection we need to figure
> +	 * out where to connect it on restore. Thus, tune up peer
> +	 * id by searching an existing listening socket.
> +	 *
> +	 * Note the socket name will be found at restore stage,
> +	 * not now, just to reduce size of dump files.
> +	 */
> +	if (!ue.peer && ue.state == TCP_ESTABLISHED) {
> +		struct unix_sk_listen_icon *e;
> +		struct unix_sk_desc *d;
> +
> +		e = lookup_unix_listen_icons(ue.id);
> +		d = e ? e->sk_desc : NULL;
> +		if (!e || !d || d->state != TCP_LISTEN) {

e = lookup_unix_listen_icons(ue.id);
if (!e) {
	pr_err("Dangling inflight");
	goto err;
}
if (e->sk_desc->state != TCP_LISTEN) { /* e->sk_desc is _always_ not NULL */
	pr_err("Broken inflight");
	goto err;
}

> +			pr_err("Dangling in-flight connection %d\n", ue.id);
> +			goto err;
> +		}
> +
> +		ue.flags	= USK_INFLIGHT;

ue.flags |= USK_INFLIGHT;

> +		ue.peer		= d->sd.ino;

ue.peer = e->sk_desc->sd.ino; /* i.e. -- no need in d variable */

> +		dprintk("\t\tunix-memo: %d (peer fixed %d)\n", ue.id, ue.peer);
> +	}
> +
>  	if (write_img(cr_fdset->fds[CR_FD_UNIXSK], &ue))
>  		goto err;
>  	if (write_img_buf(cr_fdset->fds[CR_FD_UNIXSK], sk->name, ue.namelen))
> @@ -838,7 +947,7 @@ static int open_unix_sk_stream(int sk, struct unix_sk_entry *ue, int img_fd)
>  		 * deferred to connect() later.
>  		 */
>  
> -		if (ue->peer < ue->id) {
> +		if (ue->peer < ue->id && ue->flags == USK_REGULAR) {

if ((ue->peer < ue->ud) && (ue->flags & USK_INFLIGHT))

i.e. -- explicit braces and & instead of ==, these are flags.

>  			struct sockaddr_un addr;
>  			int len;
>  			struct unix_accept_job *aj;
> @@ -877,7 +986,28 @@ static int open_unix_sk_stream(int sk, struct unix_sk_entry *ue, int img_fd)
>  			if (!cj)
>  				goto err;
>  
> -			prep_conn_addr(ue->peer, &cj->addr, &cj->addrlen);
> +			/*
> +			 * Might need to resolve in-flight connection name.
> +			 */
> +			if (ue->flags == USK_INFLIGHT) {

Same here -- flags & USK_INFLIGHT.

> +				struct unix_sk_listen *e;
> +
> +				/*
> +				 * Find out listening sockets name.
> +				 */
> +				e = lookup_unix_listen(ue->peer);
> +				if (!e) {
> +					pr_err("Bad in-flight socket peer %d\n",
> +						ue->peer);
> +					goto err;
> +				}
> +
> +				memcpy(&cj->addr, &e->addr, sizeof(cj->addr));
> +				cj->addrlen = e->addrlen;
> +			} else {
> +				prep_conn_addr(ue->peer, &cj->addr, &cj->addrlen);
> +			}

No { } around one-liners please.

> +
>  			cj->fd = ue->fd;
>  			cj->next = conn_jobs;
>  			conn_jobs = cj;