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

[Pavel Emelyanov]

On 07/20/2012 11:55 AM, Cyrill Gorcunov wrote:
> Pavel, something like below? Note, I still think we
> should continue dumping FPU state even if we don't restore
> it for a while. There is no meaning to NOT doing so.

OK let's keep it. Some more comments below

> 	Cyrill

> +message core_entry {
> +       enum march {
> +               UNKNOWN         = 0;
> +               X86_64          = 1;
> +       }
> +
> +       required march                  mtype           = 1;
> +       optional thread_info_x86        thread_info     = 2;
> +
> +       required task_core_entry        tc              = 3;
> +       required core_ids_entry         ids             = 4;
> +}

Both tc and ids MUST be optional and SHOULD NOT be dumped for threads.
Main thread MUST fail restore if these fields are missing.
(The thread_info MUST be present on restore, but MUST remain optional.)

> +message arch_x86_entry {
> +       required user_x86_regs_entry    gpregs          = 1;
> +       required user_x86_fpregs_entry  fpregs          = 2;
> +}
> +...
> +message thread_info_x86 {
> +       required arch_x86_entry         arch_x86        = 1;
> +       required uint64                 clear_tid_addr  = 2;
> +}

Don't see much point in intermediate message. I.e. thread_info_x86 = { user_x86_regs, _fpregs and clear_tid_addr }

> @@ -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;

Should be ThreadInfoX86Entry actually.

>  } __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;

Should be ThreadInfoX86Entry and TaskCoreEntry (with comm pointer pointing to on-args variable.

>  } __aligned(sizeof(long));
> 
>  struct pt_regs {

> +message core_ids_entry {
> +       required uint32                 vm_id           = 1;
> +       required uint32                 files_id        = 2;
> +       required uint32                 fs_id           = 3;
> +       required uint32                 sighand_id      = 4;
> +}
> +

IMO task_kobj_ids_entry name is better.

Thanks,
Pavel