[Devel] Re: [PATCH] [RFC] Add c/r support for listening INET sockets

[Dan Smith]

>> +	if ((h->sock_common.family == AF_INET) &&
>> +	    (h->sock.state != TCP_LISTEN)) {
>> +		/* Temporary hack to enable restore of TCP_LISTEN sockets
>> +		 * while forcing anything else to a closed state
>> +		 */
>> +		sock->sk->sk_state = TCP_CLOSE;

SH> I'll admit by this point I'm a bit confused about the order in
SH> which these things are restored.  Does do_sock_restore() run
SH> before inet_restore() below (curious whether its check for
SH> TCP_CLOSE will catch these particular sockets).

It runs after inet_restore(), which was called above via
sock->ops->restore().  Thus, inet_restore() can largely ignore the
connected sockets and then they're forced closed here (since
inet_restore() runs before the common restore routine, otherwise it
could set them closed itself).

>> +int inet_checkpoint(struct ckpt_ctx *ctx, struct socket *sock)
>> +{
>> +	struct ckpt_hdr_socket_inet *in;
>> +	int ret = -EINVAL;
>> +
>> +       in = ckpt_hdr_get_type(ctx, sizeof(*in), CKPT_HDR_SOCKET_INET);
>> +       if (!in)
>> +               goto out;

SH> Better to just return...  I don't think this'll cause a crash (it
SH> only makes ckpt_hdr_put() pass ((struct ckpt_hdr *)NULL)->len to
SH> _ckpt_hdr_put which then ignores it and does a safe kfree(NULL)),
SH> but it won't be safe if the implementation of the ckpt_hdr_*()
SH> calls end up changing.

Sure.  Also note my broken whitespace that came as a result of me
cutting patches into pieces.  I'll fix that too.

-- 
Dan Smith
IBM Linux Technology Center
email: danms at us.ibm.com
_______________________________________________
Containers mailing list
Containers at lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers