[CRIU] Re: [PATCH 6/6] protobuf: Convert sk_packet_entry to PB engine

[Pavel Emelyanov]

On 07/13/2012 09:05 PM, Cyrill Gorcunov wrote:
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
> ---
>  include/image.h          |    6 ---
>  protobuf/Makefile        |    1 +
>  protobuf/sk-packet.proto |    4 ++
>  sk-queue.c               |   77 +++++++++++++++++++++++++--------------------
>  4 files changed, 48 insertions(+), 40 deletions(-)
>  create mode 100644 protobuf/sk-packet.proto
> 

> @@ -120,15 +123,32 @@ static int dump_one_unix_fd(int lfd, u32 id, const struct fd_parms *p)
>  
>  	BUG_ON(sk->sd.already_dumped);
>  
> +	fown.uid	= p->fown.uid;
> +	fown.euid	= p->fown.euid;
> +	fown.signum	= p->fown.signum;
> +	fown.pid_type	= p->fown.pid_type;
> +	fown.pid	= p->fown.pid;

You have helper for that already, don't you?

> +	skopts.n_so_rcv_tmo	= 2;
> +	skopts.n_so_snd_tmo	= 2;

2nd attempt: these should go to sockopt dumping code.

> +	skopts.so_snd_tmo	= xmalloc(pb_repeated_size(&skopts, so_snd_tmo));
> +	skopts.so_rcv_tmo	= xmalloc(pb_repeated_size(&skopts, so_rcv_tmo));
> +
> +	if (!skopts.so_snd_tmo || !skopts.so_rcv_tmo)
> +		goto err;
> +
> +	ue.name.len	= (size_t)sk->namelen;
> +	ue.name.data	= (void *)sk->name;
> +
>  	ue.id		= id;
>  	ue.ino		= sk->sd.ino;
>  	ue.type		= sk->type;
>  	ue.state	= sk->state;
> -	ue.namelen	= sk->namelen;
>  	ue.flags	= p->flags;
>  	ue.backlog	= sk->wqlen;
>  	ue.peer		= sk->peer_ino;
> -	ue.fown		= p->fown;
> +	ue.fown		= &fown;
> +	ue.opts		= &skopts;
>  	ue.uflags	= 0;
>  
>  	if (ue.peer) {

> @@ -766,16 +776,18 @@ int collect_unix_sockets(void)
>  			if (ui->name[0] != '\0' &&
>  			    !(ui->ue->uflags & USK_EXTERN))
>  				unlink(ui->name);
> -		} else
> -			ui->name = NULL;
> +		}

Wrong. ui->name is not NULL after xmalloc.

>  
>  		ui->peer = NULL;
>  		ui->flags = 0;
>  		pr_info(" `- Got 0x%x peer 0x%x\n", ui->ue->ino, ui->ue->peer);
>  		file_desc_add(&ui->d, ui->ue->id, &unix_desc_ops);
>  		list_add_tail(&ui->list, &unix_sockets);
> +
> +		ui = NULL;

What for?

> +int pb_restore_socket_opts(int sk, SkOptsEntry *soe)
> +{
> +	int ret = 0;
> +
> +	ret |= restore_opt(sk, SO_SNDBUFFORCE, &soe->so_sndbuf);
> +	ret |= restore_opt(sk, SO_RCVBUFFORCE, &soe->so_rcvbuf);
> +
> +	if (soe->n_so_snd_tmo > 1)

if != 2 report error

> +		ret |= do_restore_opt(sk, SO_SNDTIMEO, soe->so_snd_tmo,
> +				      pb_repeated_size(soe, so_snd_tmo));
> +	else
> +		ret |= -1;
> +	if (soe->n_so_rcv_tmo > 1)
> +		ret |= do_restore_opt(sk, SO_RCVTIMEO, soe->so_rcv_tmo,
> +				      pb_repeated_size(soe, so_rcv_tmo));
> +	else
> +		ret |= -1;
> +
> +	return ret;
> +}

options dump/restore -- should be separate patch

>  	}