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

[Cyrill Gorcunov]

On Fri, Jan 27, 2012 at 08:02:09PM +0400, Pavel Emelyanov wrote:
> On 01/27/2012 07:56 PM, Cyrill Gorcunov wrote:
> > On Fri, Jan 27, 2012 at 07:41:58PM +0400, Pavel Emelyanov wrote:
> >>> +	u8	status;
> >>
> >> u8 flags;
> >>
> >> #define USK_INFLIGHT	0x1
> >>
> >> etc...
> >>
> > 
> > ok
> > 
> >>>  
> >>>  static struct socket_desc *sockets[SK_HASH_SIZE];
> >>> -__gen_static_lookup_func(struct socket_desc, lookup_socket, sockets, ino, int, ino);
> >>> +__gen_static_lookup_func(struct socket_desc, lookup_socket, sockets, ino, unsigned int, ino);
> >>
> >> Why do you change int into unsigned int? "While I'm at it" changes should go in separate patches.
> >>
> > 
> > ok
> > 
> >>> +
> >>> +static struct unix_sk_memo *unix_memos[SK_HASH_SIZE];
> >>> +__gen_static_lookup_func(struct unix_sk_memo, lookup_unix_memo, unix_memos, ino, unsigned int, ino);
> >>
> >> This hashtable should be called unix_listeners.
> >> Why can't we put the existing socket desc here? Why yet another unix_sk_memo?
> >>
> > 
> > Because it's different types. This hash serves 2 purposes -- at checkpoint type
> > it collects listeners _and_ icons, while at restore type it collects listeners
> > only. So plain socket_desc is not enought here. Moreover when you read unix
> > entries from image you save it in local variable and then create connect-job
> > if needed but I have to carry every-single listening socket read from disk
> > and can free it after connect-job complete.
> > 
> > Thus I thought having one separate structure and hash over it -- would be
> > easier than anything else.
> 
> Сделай отдельный hashtable для listen сокетов. Только не для всех подряд listen,
> а для тех, которые оригинально, из image, listen.
> 
> Потом логика будет простой.
> 
> dump:
> if (sk->peer == 0 && sk->state == TCP_ESTABLISHED) {
> 	/* inflight */
> 	l = lookup_listener_with_icons(sk->id); /* проходим по хешу и ищем */
> 	if (l == NULL)
> 		return -1; /* dangling inflight? */
> 	sk->peer = l->id;
> 	sk->flags = INFLIGHT;
> }

У меня на дампе не просто listen сокеты, а вот так

	if (socket-has-icons)
		collect-every-icon-id-into-hash
			the-hash-entry-has-associated-socket-id

т.е. у меня сразу находится -- если есть icon, то по нему можно
узнать к какому сокету он прицеплен, TCP состояние не отслеживается
на этом этапе.

> 
> restore:
> if (sk->state == TCP_LISTEN)
> 	add_to_hash(sk, usk_listeners);
> else if (sk->flags & INFLIGHT)
> 	l = lookup_listener_by_id(sk->peer);
> 	run_connect_job(sk, l);

тут нельзя просто в хэш добавить -- имя сокета сидит на стеке,
поэтому у меня структура

struct unix_sk_memo {
	union {
		struct {
			struct sockaddr_un	addr;
			unsigned int		addrlen;
		};
		void				*p;	/* arbitrary info */
	};
	unsigned int				ino;
	struct unix_sk_memo			*next;
};

при дампе там собираются

	->p = unix_sk_desc

а при ресторе там хранятся имена сокетов

	->addr и ->addrlen

я могу имя поменять на usk_listeners, но сути это сильно не изменит.
Подумаю...

	Cyrill