[CRIU] Re: [PATCH 13/14] pstree: Migrate SIDs/PGIDs if requested

[Pavel Emelyanov]

On 10/06/2012 11:26 PM, Cyrill Gorcunov wrote:
> On Sat, Oct 06, 2012 at 10:58:42AM +0400, Cyrill Gorcunov wrote:
>>
>> In case if user asked us to migrate session we need to
>> walk over all process entries and update SIDs and PGIDs
>> where needed.
> ---
> 
> Guys, here is updated variant. Actually I'm not sure
> if anyone received the whole series since at least
> Andrew has reported that he didn't receive anything
> due to openvz.org MX problems.
> 


> 
> +struct pstree_opts {
> +	bool			root_inherit_sid;
> +	bool			root_inherit_pgid;
> +
> +	pid_t			new_sid;
> +	pid_t			new_pgid;
> +};
> +

Why separate structure for new opts?

> 

And I don't see any hunks on dump-stage checking that the sid/pgid
is not leaked outside of the dumping subtree.

> +int pstree_inherit(struct pstree_opts *opts)
> +{
> +	struct pstree_item *pi;
> +	pid_t old_sid, old_pgid;
> +
> +	BUG_ON(!opts);
> +
> +	if (!opts->root_inherit_sid && !opts->root_inherit_pgid)
> +		return 0;
> +
> +	pr_info("SID/PGID inheritance PID %d (SID %d -> %d PGID %d -> %d)\n",
> +		root_item->pid.virt, root_item->sid, opts->new_sid,
> +		root_item->pgid, opts->new_pgid);
> +
> +	old_sid  = root_item->sid;
> +	old_pgid = root_item->pgid;
> +
> +#define SID_PGID_UPDATE(_pi, _old_sid, _old_pgid)		\
> +	do {							\
> +		if (opts->root_inherit_pgid) {			\
> +			if (_pi->pgid == _old_pgid)		\
> +				_pi->pgid = opts->new_pgid;	\
> +		}						\
> +								\
> +		if (opts->root_inherit_sid) {			\
> +			if (_pi->sid == _old_sid)		\
> +				_pi->sid = opts->new_sid;	\
> +			if (_pi->pgid == _old_sid)		\
> +				_pi->pgid = opts->new_sid;	\
> +		}						\
> +	} while (0)
> +
> +	if (opts->root_inherit_pgid)
> +		root_item->pgid = opts->new_pgid;
> +
> +	if (opts->root_inherit_sid) {
> +		root_item->sid = opts->new_sid;
> +		if (root_item->pgid == old_sid)
> +			root_item->pgid = opts->new_sid;
> +	}
> +
> +	for_each_pstree_item(pi)
> +		SID_PGID_UPDATE(pi, old_sid, old_pgid);
> +
> +#undef SID_PGID_UPDATE
> +
> +	return 0;
> +}
> +

If it was properly fixed on the dump stage, you wouldn't require this thing.