Re: [RFC][PATCH 7/7][v2] Define clone_with_pids syscall

[Oren Laadan <orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>]

Sukadev Bhattiprolu wrote:
> Oren Laadan [orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org] wrote:
> | 
> | 
> | Sukadev Bhattiprolu wrote:
> | > Sukadev Bhattiprolu [sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org] wrote:
> | > | Oren Laadan [orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org] wrote:
> | > 
> | > I am not sure what the semantics should be for this case:
> | > 
> | > 	- checkpoint a process that is in level-3 pid namespace
> | > 	- restart in a level-2 or level-1 pid namespace
> | > 
> | 
> | Meaning: a container root was at level-3, so tasks in the container
> | were level-3 through level-(3+N), where N is the in-container depth
> | so to speak. Then it was restarted such that the base became level-2
> | or level-1.
> | 
> | I think we already covered this.
> | 
> 
> Sorry, I meant level3 not 'level minus 3' :-) Restating, suppose
> init_pid_ns is L0, and L3 is a pid namespace 3 levels deep (i.e the
> process has 4 pids at checkpoint time). When restarting, if the
> process only needs 2 pids bc it is L1, current behavior is to return
> -EINVAL. 
> 
> I have this check in copy_target_pids():
> 
> +       if (num_pids < 0 || num_pids > nesting)
> +               return ERR_PTR(-EINVAL);
> 
> Is it ok to return -EINVAL when num_pids > nesting ?
> 

IMHO yes. I'd keep the semantics this way.

clone_with_pid() is anyway intended for restart() only, and restart
need never get to that situation. In fact, checkpoint() should only
record those pids from the container "base" downwards, so the depth
of the original "base" is irrelevant.

Besdies, the alternative is to silently ignore part of what the
caller requests - which is worse.

Oren.