[CRIU] [PATCH 5/6] files: Add c/r for /proc/$pid/ns/$ids references

[Pavel Emelyanov]

On 05/15/2013 01:02 AM, Cyrill Gorcunov wrote:
> On Wed, May 15, 2013 at 12:47:32AM +0400, Pavel Emelyanov wrote:
>>>
>>> +	switch (entry->d->type) {
>>> +	case NS_TYPES__NET:
>>> +	case NS_TYPES__UTS:
>>> +	case NS_TYPES__IPC:
>>> +	case NS_TYPES__PID:
>>> +	case NS_TYPES__MNT:
>>> +		break;
>>> +	default:
>>
>> entry->d points somewhere into the ns_desc_array. What coincidence of circumstances
>> can lead to this default: case to occur?
> 
> If we got reference to proc/ns/user. We dont support it yet, 

Why? What's wrong with it?

> while still
> it's valid reference our engine can parse and provide us back a pointer
> to array. As only we support it -- we simply drop this case. Another
> potential problem -- someone extend array of namspaces but forget to

Then we kick this guy's ass.

> update this 'case' statement, thus we get error on dump and developer
> will be prompted to fix the code.
> 
>>
>>> +		pr_err("Namespace `%s' referred by fd %d "
>>> +		       "(pid %d) is unsupported\n",
>>> +		       entry->d->str, p->fd, p->pid);
>>> +		return -1;
>>> +	}
>>
>>> +	for_each_pstree_item(t) {
>>> +		TaskKobjIdsEntry *ids = t->ids;
>>> +
>>> +		if (ids->has_pid_ns_id && ids->pid_ns_id == nfi->nfe->ns_id) {
>>
>> How can we have images with FD_TYPES__NS files but with pstree items without the has_xxx_ns_id?
> 
> I think if the protobuf has optional field and we "can" check for
> it presense -- we should. It's absolutely valid situation when
> someone edit image manually,

OK. In this case absense of ns_ids in pstree should be treated as error.

> drops has_pid_ns_id and set both
> pid_ns_id and ns_id to same meaning (say zero), and I think
> we better should yield error here than assuming it's always
> valid. Or I miss something?
> 
>>> +int collect_ns_files(void)
>>> +{
>>> +	return collect_image(CR_FD_NS_FILES, PB_NS_FILES,
>>> +			     sizeof(struct ns_file_info),
>>> +			     collect_one_nsfile);
>>
>> v0.5 crtools might bring us images w/o this file.
> 
> Thanks, i'll update.
> .
>