[CRIU] Re: [PATCH 12/14] tty: Make tty_get_sid_pgrp() to use parasite code

[Cyrill Gorcunov]

On Mon, Oct 15, 2012 at 08:06:26PM +0400, Pavel Emelyanov wrote:
> On 10/15/2012 06:54 PM, Cyrill Gorcunov wrote:
> > 
> > We need to use namespace local sid/pgid parameters
> > assigned on tty link, thus use parasite helper.
> 
> How does current code works in container? With BUGs?

I think it works a bit incorrectly.

> > Otherwise we might fail on restore. This patch as
> > well takes into account if user asked to migrate
> > the tty connection.
> > 
> > Basically the idea is simple -- if we have some
> > tty peer with some SID and the SID is not presented
> > in our pstree -- we either were asked to migrate,
> > either user typed a wrong PID and we can't continue.
> > 
> > Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
> > ---
> >  tty.c |   78 ++++++++++++++++++++++++++++++++++++++++++++++------------------
> >  1 files changed, 56 insertions(+), 22 deletions(-)
> > 
> 
> 
> > +	 *
> > +	 * In this case we simply zap sid/pgid and inherit
> > +	 * the peer from the current terminal on restore.
> > +	 */
> > +	if (args.sid) {
> > +		struct pstree_item *item = find_first_sid(args.sid);
> > +		if (!item || item->pid.virt != args.sid) {
> > +			if (!opts.shell_job) {
> > +				pr_err("Found sid %d pgid %d on slave peer fd %d. "
> > +				       "Missing to pass %s option?\n",
> > +				       args.sid, args.pgrp, p->fd, "--shell-job");
> > +				return -1;
> > +			}
> > +
> > +			args.sid = args.pgrp = 0;
> 
> So zero sid/pgid on terminal means "inherit from current"?

Not exactly. When sid/pgid fetched from slave peer -- it means we're
session leaders thus we should be present in process tree, in this case
we write proper sid into image. But if the session leader was not found
in process tree -- it means we've been asked to migrate, thus we write
0s here (well, it seems better to write one of INHERIT_SID value here)
and when we restore the program the tty verifucation code will check that
this slave peer is single external peer and it doesn't matter which
sid/pgid it had since we will be inheriting tty anyway.

But I agree it looks fishy. I'll update.