[CRIU] Re: [PATCH 6/9] pstree: Allow to dump and restore session non-leaders if --ext-tty passed

[Pavel Emelyanov]

On 10/09/2012 07:40 PM, Cyrill Gorcunov wrote:
> 
> If --ext-tty passed we allow to dump and restpre session non-leaders.
> 
> This slightly changes the behaviour or the tool. If previously
> we allowed to dump non-leaders after this commit we refuse to
> do that if not explicitly requested.
> 
> Note that in sake of tty restore (which will be addressed in
> further patches) we do inherit process group for root task.
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
> ---
>  cr-dump.c        |    2 +-
>  include/pstree.h |    9 ++++++-
>  pstree.c         |   66 +++++++++++++++++++++++++++++++++++++++++++++++++++--
>  3 files changed, 72 insertions(+), 5 deletions(-)
> 


> @@ -1614,7 +1614,7 @@ int cr_dump_tasks(pid_t pid, const struct cr_options *opts)
>  	if (dump_zombies())
>  		goto err;
>  
> -	if (dump_pstree(root_item))
> +	if (dump_pstree(root_item, opts))
>  		goto err;
>  
>  	if (opts->namespaces_flags)

The opts is a good global variable, no need in passing it all over the stack.

> +	/*
> +	 * Make sure we're dumping session leader, if not an
> +	 * appropriate option must be passed. Another ruse is
> +	 * that to restore external tty connections we need to
> +	 * inherit process group as well, thus zap it here.
> +	 */
> +	if (root_item->pid.virt != root_item->sid) {

I think we need a comment, why root's session leader can't sit somewhere
in the subtree _below_ it.

> +		if (item->pgid == old_sid)
> +			e.pgid = INHERIT_SID;
> +		else if (item->pgid == old_pgid)
> +			e.pgid = INHERIT_PGID;
> +		else
> +			e.pgid = item->pgid;

I don't understand this.

> @@ -148,6 +204,7 @@ int prepare_pstree(void)
>  			}
>  			root_item = pi;
>  			pi->parent = NULL;
> +
>  		} else {
>  			/*
>  			 * Fast path -- if the pstree image is not edited, the

Trash

> @@ -331,6 +388,9 @@ int prepare_pstree_ids(void)
>  		if (gleader)
>  			continue;
>  
> +		if (current_pgid == item->pgid)
> +			continue;
> +
>  		helper = alloc_pstree_item();
>  		if (helper == NULL)
>  			return -1;

Please, explain this hunk.