[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