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

Cyrill Gorcunov gorcunov at openvz.org
Tue May 14 17:02:13 EDT 2013


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


More information about the CRIU mailing list