[CRIU] Re: Before we do the CRIU v0.1 release

[Pavel Emelyanov]

On 07/19/2012 08:10 PM, Cyrill Gorcunov wrote:
> Here is updated.
> 
> 	Cyrill

OK. Let's do a reshuffle of the patch

1.

> +message arch_x86_entry {
> +	required user_x86_regs_entry	gpregs		= 1;
> +	required user_x86_fpregs_entry	fpregs		= 2;

I see only dump of this stuff. Why do we carry this at all?

> +}
> +
> +message task_core_entry {
> +	required uint32			task_state	= 1;
> +	required uint32			exit_code	= 2;
> +
> +	required uint32			personality	= 3;
> +	required uint32			flags		= 4;
> +	required uint64			blk_sigset	= 5;
> +
> +	required string			comm		= 6;
> +}
> +
> +message core_ids_entry {
> +	required uint32			vm_id		= 1;
> +	required uint32			files_id	= 2;
> +	required uint32			fs_id		= 3;
> +	required uint32			sighand_id	= 4;
> +}
> +
> +message core_entry {
> +	enum march {
> +		UNKNOWN		= 0;
> +		X86_64		= 1;
> +		X86_32		= 2;

Drop it for now. Only unknown and x64 should be left.

> +	}
> +
> +	required uint32			version		= 1;

Do we REALLY need this here? IMO it was a mistake to leave this field in
the per-task core image. If we want the images version to be it should either
sit in every file, or be in the root file -- pstree.img, or we should have
a bounding image file titled description.img or smth like this. I'd vote
for the last scenario.

> +	required march			mtype		= 2;
> +
> +	required task_core_entry	tc		= 3;
> +	required core_ids_entry		ids		= 4;
> +	required uint64			clear_tid_addr	= 5;
> +
> +	optional arch_x86_entry		arch_x86	= 6;
> +}

2.

> @@ -60,15 +61,15 @@ struct thread_restore_args {
>  	struct restore_mem_zone		mem_zone;
>  
>  	int				pid;
> -	int				fd_core;
>  	mutex_t				*rst_lock;
> +	UserX86RegsEntry		gpregs;
> +	u64				clear_tid_addr;
>  } __aligned(sizeof(long));
>  
>  struct task_restore_core_args {
>  	struct restore_mem_zone		mem_zone;
>  
>  	int				pid;			/* task pid */
> -	int				fd_core;		/* opened core file */
>  	int				fd_exe_link;		/* opened self->exe file */
>  	int				fd_pages;		/* opened pages dump file */
>  	int				logfd;
> @@ -95,6 +96,11 @@ struct task_restore_core_args {
>  
>  	MmEntry				mm;
>  	u64				mm_saved_auxv[AT_VECTOR_SIZE];
> +	u64				clear_tid_addr;
> +	u64				blk_sigset;
> +	char				comm[TASK_COMM_LEN];
> +	CoreIdsEntry			ids;
> +	UserX86RegsEntry		gpregs;
>  } __aligned(sizeof(long));
>  
>  struct pt_regs {

Thus the core entry consists of 2 items.

a) parts, that are individual for every thread
b) part relevant for the process as a whole

That said, let's declare the message core_entry like this

message x864_64_thread_info {
	/* registers */
	/* clear tid address */
}

message core_entry {
	required march march;
	optional x86_64_thread_info thread_info
	/* process-wide stuff */
}

3. The master thread core entry isn't get __free_unpacked-ed. Leak?