[CRIU] Re: [PATCH 6/7] sockets: Restore unbound inet sockets

[Cyrill Gorcunov]

On Wed, May 16, 2012 at 12:56:24AM +0400, Pavel Emelyanov wrote:
> > +
> > +#define __CR_GET_SOPT(lfd, n, v)					\
> > +	({								\
> > +		len = sizeof(*v);					\
> > +		int __r = getsockopt(lfd, SOL_SOCKET, n, v, &len);	\
> > +		if (__r)						\
> > +			pr_perror("getsockopt failed on %d", p->fd);	\
> > +		__r;							\
> > +	})
> 
> Use do_dump_opt from sockets.c file. They do this error reporting and
> also check option length to be correct.

OK.

> > +	default:
> > +		pr_err("Unsupported socket type %d on %d\n",
> > +		       sk->type, p->fd);
> > +		goto err;
> > +	}
> 
> This check is done in generic inet socket dump

Yes, this is self protection (ie when we will be changing this code
in future -- make sure we're not breaking it). So I guess I better
should use BUG_ON(1) here.

> > +	if (sk->sd.family == AF_INET) {
> > +		len = sizeof(addr.v4);
> > +		ret = getsockname(lfd, (struct sockaddr *)&addr.v4, &len);
> 
> This requires an explanation. If a socket has a name, this means it's bound,
> but in this case it should have been reported via netlink.
> 

Hmm, indeed, seems I overdone here. Thanks!

> > +		if (ret) {
> > +			if (errno != ENOTCONN) {
> > +				pr_perror("getsockname failed on %d", p->fd);
> > +				goto err;
> > +			}
> > +		} else {
> > +			sk->src_port = addr.v4.sin_port;
> > +			memcpy(sk->src_addr, &addr.v4.sin_addr.s_addr, sizeof(addr.v4.sin_addr.s_addr));
> > +		}
> > +
> > +		len = sizeof(addr.v4);
> > +		ret = getpeername(lfd, (struct sockaddr *)&addr.v4, &len);
> 
> Same here -- connected sockets are hashed and are reported via netlink. Aren't they?

Yes, bound socks should be reported via netlink. Thanks for catching.

	Cyrill