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

[Pavel Emelyanov]

On 07/20/2012 12:23 PM, Cyrill Gorcunov wrote:
> On Fri, Jul 20, 2012 at 12:14:02PM +0400, Pavel Emelyanov wrote:
>>> +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.)
> 
> OK
> 
>>> +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 }
> 
> OK
> 
>>> @@ -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.
> 
> No. ThreadInfoX86Entry will have pointers to user_x86_regs and such which
> is forbitten here, we provide own memory tp carry this entries and
> we need to "copy by values". I mean that usually this thread_info_x86
> will look like (on C level)
> 
> struct thread_info_x86 {
> 	user_x86_regs *p;
> 	...
> }
> 
> and of course I can't refer to "p" from inside restorer resident code,
> instead i need fully embedded user_x86_regs structure so that I can
> read every register value from restorer itself.
> 
> But if you still prefer to see ThreadInfoX86Entry here I need to
> put contents of dereferenced ThreadInfoX86Entry members somewhere
> on restorer heap and tune up pointers value. Should I?

Ah I see. Well, OK, let's keep the innermost entries here only.

>>>         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.
> 
> Hmm, ok.

Well, same here then.

>> IMO task_kobj_ids_entry name is better.
> 
> Sure.
> 
> 	Cyrill
> .
>