[CRIU] [PATCH v2 5/5] compel: Add nspid, nssid and nspgid fields to struct seize_task_status

[Kirill Tkhai]

On 02.05.2017 14:36, Kirill Tkhai wrote:
> On 02.05.2017 13:06, Pavel Emelyanov wrote:
>> On 05/02/2017 12:18 PM, Kirill Tkhai wrote:
>>> On 02.05.2017 11:58, Pavel Emelyanov wrote:
>>>> On 04/29/2017 09:12 AM, Andrei Vagin wrote:
>>>>> On Fri, Apr 28, 2017 at 11:03:08PM -0700, Andrei Vagin wrote:
>>>>>> On Tue, Apr 18, 2017 at 06:57:07PM +0300, Kirill Tkhai wrote:
>>>>>>> Some users want them, so add these fields to the structure.
>>>>>>>
>>>>>>> Signed-off-by: Kirill Tkhai <ktkhai at virtuozzo.com>
>>>>>>> ---
>>>>>>>  compel/include/uapi/infect.h |    4 ++++
>>>>>>>  compel/src/lib/infect.c      |    6 ++++++
>>>>>>>  2 files changed, 10 insertions(+)
>>>>>>>
>>>>>>> diff --git a/compel/include/uapi/infect.h b/compel/include/uapi/infect.h
>>>>>>> index 7b681b0cb..4c6ab413d 100644
>>>>>>> --- a/compel/include/uapi/infect.h
>>>>>>> +++ b/compel/include/uapi/infect.h
>>>>>>> @@ -21,6 +21,10 @@ struct seize_task_status {
>>>>>>>  	int			seccomp_mode;
>>>>>>>  	unsigned long long	shdpnd;
>>>>>>>  	unsigned long long	sigpnd;
>>>>>>> +	size_t			ns_levels;
>>>>>>> +	uint32_t		*nspid;
>>>>>>> +	uint32_t		*nspgid;
>>>>>>> +	uint32_t		*nssid;
>>>>>>
>>>>>> Here is the compel code and now we try to add fields which are
>>>>>> not used in compel. It doesn't look good.
>>>>>
>>>>> Maybe we need to declare a new structure in criu which will encapsulate
>>>>> seize_task_status?
>>>>
>>>> We already have one ;) Called proc_status_creds.
>>>
>>> struct proc_status_creds is a mix entities of different types collected
>>> from different files. seize_task_status is the only structure for
>>> /proc/[pid]/status file, all content of this structure is collected
>>> from the single file. It's different things.
>>
>> proc_status_creds is also filled only with values collected from /proc/pid/status,
>> nothing more.
> 
> But it has only statically stored values, and it has no problems like the problems
> solved in this patchset.

We may continue the way suggested via proc_status_creds and also allocate

uint32_t nspid[32];
uint32_t nspgid[32];
uint32_t nssid[32];

in the same structure, but let's introduce the generic memory-efficient solution.
  
>>>>>>
>>>>>>
>>>>>>>  	/* Add new members to the bottom and do not change existing */
>>>>>>>  };
>>>>>>>  
>>>>>>> diff --git a/compel/src/lib/infect.c b/compel/src/lib/infect.c
>>>>>>> index 9ade844cf..ab7558a81 100644
>>>>>>> --- a/compel/src/lib/infect.c
>>>>>>> +++ b/compel/src/lib/infect.c
>>>>>>> @@ -190,11 +190,17 @@ static int skip_sigstop(int pid, int nr_signals)
>>>>>>>  /* Init dynamically allocated fields in NULL and do not touch other */
>>>>>>>  static void init_seize_task_status(struct seize_task_status *ss)
>>>>>>>  {
>>>>>>> +	ss->ns_levels = 0;
>>>>>>> +	ss->nspid = ss->nspgid = ss->nssid = NULL;
>>>>>>>  }
>>>>>>>  
>>>>>>>  /* Free dynamically allocated fields in compel_wait_task() and do not touch other */
>>>>>>>  void compel_consume_seize_task_status(struct seize_task_status *ss)
>>>>>>>  {
>>>>>>> +	xfree(ss->nspid);
>>>>>>> +	xfree(ss->nspgid);
>>>>>>> +	xfree(ss->nssid);
>>>>>>> +
>>>>>>>  	init_seize_task_status(ss);
>>>>>>>  }
>>>>>>>  
>>>>>>>
>>>>> .
>>>>>
>>>>
>>> .
>>>
>>