[CRIU] Re: [PATCH] tty: Rework tty_find_restoring_task

Cyrill Gorcunov gorcunov at openvz.org
Mon Oct 29 18:07:49 EDT 2012


On Tue, Oct 30, 2012 at 12:19:02AM +0400, Andrew Vagin wrote:
> On Mon, Oct 29, 2012 at 11:28:32PM +0400, Cyrill Gorcunov wrote:
> > It's being found that we fail to restore tasks which
> > are not session leaders but have controlling terminals.
> 
> It is not true. Sorry, I made this mess in our heads.

Pavel, Andrew, here are two patches for this nit.
-------------- next part --------------
>From 4f762d7c59d3b08cd892810eafa7f2df7594e6d0 Mon Sep 17 00:00:00 2001
From: Cyrill Gorcunov <gorcunov at openvz.org>
Date: Mon, 29 Oct 2012 23:15:23 +0400
Subject: [PATCH 1/2] tty: Rework tty_find_restoring_task

It's being found that we fail to restore tasks which
are not session leaders but have controlling terminals.

This patch reworks tty_find_restoring_task logic to fix
the problem. The problem itself basically was the following
-- in case if there no sid present even on master peer or
on both master/slave peers, we were trying to find a restoring
task, and that's wrong since we have peers which do not belong
our session.

https://bugzilla.openvz.org/show_bug.cgi?id=2409

Reported-by: Andrey Vagin <avagin at openvz.org>
Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
---
 tty.c |  100 ++++++++++++++++++++++++++++++++++++++++++++++++----------------
 1 files changed, 75 insertions(+), 25 deletions(-)

diff --git a/tty.c b/tty.c
index dd78ee2..88857b5 100644
--- a/tty.c
+++ b/tty.c
@@ -395,7 +395,15 @@ static bool pty_is_master(struct tty_info *info)
 
 static bool pty_is_hung(struct tty_info *info)
 {
-	return info->tie->sid == 0 && info->tie->termios == NULL;
+	return info->tie->termios == NULL;
+}
+
+static bool tty_has_active_pair(struct tty_info *info)
+{
+	int d = pty_is_master(info) ? -1 : + 1;
+
+	return test_bit(info->tfe->tty_info_id + d,
+			tty_active_pairs);
 }
 
 static void tty_show_pty_info(char *prefix, struct tty_info *info)
@@ -678,48 +686,90 @@ static struct pstree_item *find_first_sid(int sid)
 	return NULL;
 }
 
+static struct pstree_item *tty_lookup_task_sid(struct tty_info *info)
+{
+	struct pstree_item *item;
+
+	for_each_pstree_item(item) {
+		if (item->sid == info->tie->sid)
+			return item;
+	}
+
+	return NULL;
+}
+
 static int tty_find_restoring_task(struct tty_info *info)
 {
 	struct pstree_item *item;
 
+	/*
+	 * The overall scenario is the following (note
+	 * we might have corrupted image so don't believe
+	 * anything).
+	 *
+	 * SID is present on a peer
+	 * ------------------------
+	 *
+	 *  - if it's master peer and we have as well a slave
+	 *    peer then prefer restore controlling terminal
+	 *    via slave peer
+	 *
+	 *  - if it's master peer without slave, there must be
+	 *    a SID leader who will be restoring the peer
+	 *
+	 *  - if it's a slave peer and no session leader found
+	 *    than we need an option to inherit terminal
+	 *
+	 * No SID present on a peer
+	 * ------------------------
+	 *
+	 *  - if it's a master peer than we are in good shape
+	 *    and continue in a normal way, we're the peer keepers
+	 *
+	 *  - if it's a slave peer and no appropriate master peer
+	 *    found we need an option to inherit terminal
+	 *
+	 * In any case if it's hungup peer, then we jump out
+	 * early since it will require fake master peer and
+	 * rather non-usable anyway.
+	 */
+
+	if (pty_is_hung(info)) {
+		pr_debug("Hungup terminal found id %x\n", info->tfe->id);
+		return 0;
+	}
+
 	if (info->tie->sid) {
-		/*
-		 * If there a slave peer present then it will
-		 * restore the controlling terminal.
-		 */
 		if (pty_is_master(info)) {
-			if (test_bit(info->tfe->tty_info_id - 1, tty_active_pairs))
+			if (tty_has_active_pair(info))
 				return 0;
 		}
 
-		pr_info("Set a control terminal %x to %d\n",
-			info->tfe->id, info->tie->sid);
-
-		for_each_pstree_item(item) {
-			if (item->sid == info->tie->sid)
-				return prepare_ctl_tty(item->pid.virt,
-						       item->rst,
-						       info->tfe->id);
+		item = tty_lookup_task_sid(info);
+		if (item) {
+			pr_info("Set a control terminal %x to %d\n",
+				info->tfe->id, info->tie->sid);
+			return prepare_ctl_tty(item->pid.virt,
+					       item->rst,
+					       info->tfe->id);
 		}
-	}
 
-	/*
-	 * Hung up terminals require a fake master peer.
-	 */
-	if (pty_is_hung(info)) {
-		pr_debug("Hung up terminal id %x\n", info->tfe->id);
-		return 0;
+		if (pty_is_master(info))
+			goto notask;
+	} else {
+		if (pty_is_master(info))
+			return 0;
+		if (tty_has_active_pair(info))
+			return 0;
 	}
 
-	/*
-	 * We can only inherit one slave peer.
-	 */
-	if (opts.shell_job && !pty_is_master(info)) {
+	if (opts.shell_job) {
 		pr_info("Inherit terminal for id %x\n", info->tfe->id);
 		info->tie->sid = info->tie->pgrp = INHERIT_SID;
 		return 0;
 	}
 
+notask:
 	pr_err("No task found with sid %d\n", info->tie->sid);
 	return -1;
 }
-- 
1.7.7.6

-------------- next part --------------
>From f98860fa88292b4b26e82e380db1ead71f60345e Mon Sep 17 00:00:00 2001
From: Cyrill Gorcunov <gorcunov at openvz.org>
Date: Tue, 30 Oct 2012 02:03:42 +0400
Subject: [PATCH 2/2] tty: Open slave peers with O_NOCTTY

We have own mechanism to restore controlling
terminal thus we don't need the kernel to create
it in implicit way.

Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
---
 tty.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tty.c b/tty.c
index 88857b5..f087ba8 100644
--- a/tty.c
+++ b/tty.c
@@ -472,7 +472,7 @@ static int pty_open_slaves(struct tty_info *info)
 	list_for_each_entry(slave, &info->sibling, sibling) {
 		BUG_ON(pty_is_master(slave));
 
-		fd = open(pts_name, slave->tfe->flags);
+		fd = open(pts_name, slave->tfe->flags | O_NOCTTY);
 		if (fd < 0) {
 			pr_perror("Can't open slave %s", pts_name);
 			goto err;
-- 
1.7.7.6



More information about the CRIU mailing list