[CRIU] Re: [PATCH 2/4] tty: Add checkpoint/restore for unix terminals v3

Cyrill Gorcunov gorcunov at openvz.org
Fri Sep 7 13:19:24 EDT 2012


On Fri, Sep 07, 2012 at 06:49:18PM +0400, Pavel Emelyanov wrote:
> On 09/07/2012 06:45 PM, Cyrill Gorcunov wrote:
> > On Fri, Sep 07, 2012 at 06:39:33PM +0400, Pavel Emelyanov wrote:
> >>
> >> You assign the n_c_cc = TERMIOS_NCC, thus you should check in on restore.
> > 
> > Ah, that. I'll do a fix on top. Thanks!
> > 
> >> OK, this patch looks sane. Where's the rest with control terminals?
> > 
> > I'm cooking control terminals now (this requires more efforts than I
> > expected because I redesigned it, now control terminals should have
> > more transparent scheme of restore). Once it's done i'll send them
> > out. If you wish you can merge these patches since our test cases
> > do work with them just fine.
> 
> No, I will wait for the control ptys. I don't understand the .proto file
> layout you chose, need to see the whole picture.

OK, does the patch attached shed some light? Note I remember your complain
about walking over pstree entries looking up for the task we need, but there
not much we can do -- both tasks and ttys are dumped in different time and
restored separately so I think we rather need some hash for pstree to find
required entry fast.

	Cyrill
-------------- next part --------------
>From be038a2cd6bf0f301f7753808613e7b873110ee4 Mon Sep 17 00:00:00 2001
From: Cyrill Gorcunov <gorcunov at openvz.org>
Date: Fri, 7 Sep 2012 20:50:57 +0400
Subject: [PATCH] tty: Restore controlling terminal

Because setting up a controlling terminal is just
call for ioctl over interface (/dev/pts/N) under
proper context (where proper context means a task
with sid matched to one the terminal has), we do
some tricks

 - we dump controlling tty with unique id and very
   few parameters (that's why a number of parameters
   in tty.proto made optional)

 - at restore time we find a task which has the sid
   we need and assign ctl-tty-id there obtained in
   previous step

 - then we generate fake fdinfo entry and bind it to
   ctl-tty-id, thus when file engine finds this fd
   it'll know that something special is done inside
   open_fd handler and simply close this fd once
   open_fd finished

 - in our open_fd handled we simply yield ioctl
   on /dev/pts/N and that's all

It's important to note that since /dev/pts/N is a slave
peer an appropriate master peer will be already created
by file engine.

Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
---
 cr-dump.c         |   10 ---
 cr-restore.c      |    2 +-
 files.c           |   20 +++++-
 include/crtools.h |    1 +
 include/files.h   |    2 +-
 include/pstree.h  |    1 +
 tty.c             |  198 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 7 files changed, 214 insertions(+), 20 deletions(-)

diff --git a/cr-dump.c b/cr-dump.c
index 387a09e..855e72f 100644
--- a/cr-dump.c
+++ b/cr-dump.c
@@ -238,16 +238,6 @@ static int dump_chrdev(struct fd_parms *p, int lfd, const struct cr_fdset *set)
 	case TTYAUX_MAJOR:
 	case UNIX98_PTY_MASTER_MAJOR ... (UNIX98_PTY_MASTER_MAJOR + UNIX98_PTY_MAJOR_COUNT - 1):
 	case UNIX98_PTY_SLAVE_MAJOR:
-		/*
-		 * FIXME
-		 *
-		 * Until control terminals are restored and standalone
-		 * slaves are supported skip stdin/out/err.
-		 */
-		if (p->fd < 3) {
-			pr_info("... Skipping tty ... %d\n", p->fd);
-			return 0;
-		}
 		return dump_tty(p, lfd, set);
 	}
 
diff --git a/cr-restore.c b/cr-restore.c
index c265a45..ff6e14c 100644
--- a/cr-restore.c
+++ b/cr-restore.c
@@ -131,7 +131,7 @@ static int prepare_shared(void)
 		if (ret < 0)
 			break;
 
-		ret = prepare_fd_pid(pi->pid.virt, pi->rst);
+		ret = prepare_fd_pid(pi->pid.virt, pi->rst, pi->ctl_tty_id);
 		if (ret < 0)
 			break;
 	}
diff --git a/files.c b/files.c
index d815f5e..0bc272e 100644
--- a/files.c
+++ b/files.c
@@ -196,7 +196,7 @@ static int collect_fd(int pid, FdinfoEntry *e, struct rst_info *rst_info)
 	return 0;
 }
 
-int prepare_fd_pid(int pid, struct rst_info *rst_info)
+int prepare_fd_pid(int pid, struct rst_info *rst_info, u32 ctl_tty_id)
 {
 	int fdinfo_fd, ret = 0;
 
@@ -227,6 +227,19 @@ int prepare_fd_pid(int pid, struct rst_info *rst_info)
 		}
 	}
 
+	if (ctl_tty_id) {
+		FdinfoEntry *e = xmalloc(sizeof(*e));
+		if (!e)
+			return -1;
+		fdinfo_entry__init(e);
+
+		e->id		= ctl_tty_id;
+		e->fd		= get_service_fd(CTL_TTY_OFF);
+		e->type		= FD_TYPES__TTY;
+
+		ret = collect_fd(pid, e, rst_info);
+	}
+
 	close(fdinfo_fd);
 	return ret;
 }
@@ -359,6 +372,11 @@ static int open_fd(int pid, FdinfoEntry *fe, struct file_desc *d)
 	if (tmp < 0)
 		return -1;
 
+	if (is_service_fd(fe->fd, CTL_TTY_OFF)) {
+		close(tmp);
+		return 0;
+	}
+
 	if (reopen_fd_as(fe->fd, tmp))
 		return -1;
 
diff --git a/include/crtools.h b/include/crtools.h
index 4a464a5..c361e6b 100644
--- a/include/crtools.h
+++ b/include/crtools.h
@@ -107,6 +107,7 @@ enum {
 	IMG_FD_OFF,
 	SELF_EXE_FD_OFF,
 	PROC_FD_OFF,
+	CTL_TTY_OFF,
 
 	SERVICE_FD_OFF_MAX
 };
diff --git a/include/files.h b/include/files.h
index 2a30846..d578fb5 100644
--- a/include/files.h
+++ b/include/files.h
@@ -86,7 +86,7 @@ extern int rst_file_params(int fd, FownEntry *fown, int flags);
 extern void show_saved_files(void);
 
 extern int prepare_fds(struct pstree_item *me);
-extern int prepare_fd_pid(int pid, struct rst_info *rst_info);
+extern int prepare_fd_pid(int pid, struct rst_info *rst_info, u32 ctl_tty_id);
 extern int prepare_shared_fdinfo(void);
 extern int get_filemap_fd(int pid, VmaEntry *vma_entry);
 extern int prepare_fs(int pid);
diff --git a/include/pstree.h b/include/pstree.h
index dc7f232..e78326e 100644
--- a/include/pstree.h
+++ b/include/pstree.h
@@ -11,6 +11,7 @@ struct pstree_item {
 	pid_t			pgid;
 	pid_t			sid;
 	pid_t			born_sid;
+	u32			ctl_tty_id;
 	int			state;		/* TASK_XXX constants */
 	int			nr_threads;	/* number of threads */
 	struct pid		*threads;	/* array of threads */
diff --git a/tty.c b/tty.c
index e8ebe38..75104c9 100644
--- a/tty.c
+++ b/tty.c
@@ -278,6 +278,73 @@ static int lock_pty_master(int master)
 	return 0;
 }
 
+static int tty_get_sid(int fd)
+{
+	int sid, ret;
+
+	ret = ioctl(fd, TIOCGSID, &sid);
+	if (ret < 0) {
+		if (errno != ENOTTY) {
+			pr_perror("Can't get sid on %d", fd);
+			return -1;
+		}
+		sid = 0;
+	}
+	return sid;
+}
+
+static int tty_set_sid(int fd)
+{
+	if (ioctl(fd, TIOCSCTTY, 1)) {
+		pr_perror("Can't set sid on terminal fd %d\n", fd);
+		return -1;
+	}
+
+	return 0;
+}
+
+static int tty_get_prgp(int fd)
+{
+	int prgp, ret;
+
+	ret = ioctl(fd, TIOCGPGRP, &prgp);
+	if (ret < 0) {
+		if (errno != ENOTTY) {
+			pr_perror("Can't get prgp on %d", fd);
+			return -1;
+		}
+		prgp = 0;
+	}
+	return prgp;
+}
+
+static int tty_set_prgp(int fd, int group)
+{
+	if (ioctl(fd, TIOCSPGRP, &group)) {
+		pr_perror("Failed to set group %d on %d\n", group, fd);
+		return -1;
+	}
+	return 0;
+}
+
+int tty_restore_ctl_terminal(int fd, struct tty_file_info *info)
+{
+	pid_t sid = getsid(getpid());
+
+	BUG_ON(sid != info->tfe->sid);
+
+	pr_info("Restore session %d by %d tty on %d\n",
+		 sid, (int)getpid(), fd);
+
+	if (tty_set_sid(fd)) {
+		pr_perror("Can't set session %d by %d tty on %d",
+			  sid, (int)getpid(), fd);
+		return -1;
+	}
+
+	return tty_set_prgp(fd, info->tfe->prgp);
+}
+
 static char *tty_type(struct tty_file_info *info)
 {
 	static char *tty_types[] = {
@@ -528,23 +595,69 @@ static struct file_desc_ops tty_desc_ops = {
 	.want_transport = tty_transport,
 };
 
-static int tty_setup_slavery(void)
+static int ctl_tty_open(struct file_desc *d)
 {
-	struct tty_file_info *info, *peer, *m;
+	struct tty_file_info *info = container_of(d, struct tty_file_info, d);
+	int ctl_fd;
+
+	tty_show_pty_info("open ctl tty", info);
+
+	ctl_fd = try_open_pts(info->tfe->pty->index, O_RDONLY, true);
+	if (ctl_fd < 0)
+		return -1;
+
+	if (tty_restore_ctl_terminal(ctl_fd, info))
+		close_safe(&ctl_fd);
+
+	return ctl_fd;
+}
+
+static struct file_desc_ops ctl_tty_desc_ops = {
+	.type		= FD_TYPES__TTY,
+	.open		= ctl_tty_open,
+};
+
+static int tty_find_restoring_task(struct tty_file_info *info)
+{
+	struct pstree_item *item;
+
+	if (info->tfe->sid == 0)
+		return 0;
+
+	pr_info("Set a control terminal to %d\n", info->tfe->sid);
 
 	/*
-	 * FIXME
-	 *
-	 * Find which task should restore
-	 * control terminals.
+	 * Note we change ctl_tty_id to match one the real
+	 * control terminal fd entry has. This is done to
+	 * make sure the control terminal fake entry will
+	 * be staged into proper fdinfo list the file engine
+	 * uses.
 	 */
+	for_each_pstree_item(item) {
+		if (item->sid == info->tfe->sid) {
+			item->ctl_tty_id = info->tfe->id;
+			return 0;
+		}
+	}
+
+	pr_err("Schei?e, no task found with sid %d\n", info->tfe->sid);
+	return -1;
+}
+
+static int tty_setup_slavery(void)
+{
+	struct tty_file_info *info, *peer, *m;
 
 	list_for_each_entry(info, &all_ttys, list) {
+		tty_find_restoring_task(info);
+
 		peer = info;
 		list_for_each_entry_safe_continue(peer, m, &all_ttys, list) {
 			if (peer->tfe->pty->index != info->tfe->pty->index)
 				continue;
 
+			tty_find_restoring_task(peer);
+
 			list_add(&peer->priv.pty.sibling, &info->priv.pty.sibling);
 			list_del(&peer->list);
 		}
@@ -590,7 +703,11 @@ static int collect_one_tty(void *obj, ProtobufCMessage *msg)
 	pr_info("Collected tty ID %#x\n", info->tfe->id);
 
 	list_add(&info->list, &all_ttys);
-	file_desc_add(&info->d, info->tfe->id, &tty_desc_ops);
+
+	if (unlikely(info->tfe->ctl_tty))
+		file_desc_add(&info->d, info->tfe->id, &ctl_tty_desc_ops);
+	else
+		file_desc_add(&info->d, info->tfe->id, &tty_desc_ops);
 
 	return 0;
 }
@@ -653,6 +770,51 @@ static int pty_get_flags(int lfd, TtyFileEntry *e)
 	return 0;
 }
 
+/*
+ * For control terminals we only need a few parameters
+ * dumped but in a sake of simplicity we don't produce
+ * new .proto entry for it and just reuse already
+ * existing format.
+ */
+static int dump_control_pty(int index, pid_t sid, pid_t prgp)
+{
+	TtyFileEntry e = TTY_FILE_ENTRY__INIT;
+	TtyPtyEntry pty = TTY_PTY_ENTRY__INIT;
+	struct stat stat;
+	char path[64];
+	int fd;
+
+	pr_info("Dump control terminal for %d\n", sid);
+
+	snprintf(path, sizeof(path), PTS_FMT, index);
+
+	fd = open(path, O_RDONLY);
+	if (fd < 0) {
+		pr_err("Can't open terminal %s\n", path);
+		return -1;
+	}
+
+	if (fstat(fd, &stat) < 0) {
+		pr_perror("Can't fstat tty %s", path);
+		close(fd);
+		return -1;
+	}
+
+	close(fd);
+
+	pty.index	= index;
+
+	e.id		= fd_id_generate_special();
+	e.rdev		= stat.st_rdev;
+	e.sid		= sid;
+	e.prgp		= prgp;
+	e.pty		= &pty;
+	e.type		= TTY_TYPE__PTY;
+	e.ctl_tty	= true;
+
+	return pb_write_one(fdset_fd(glob_fdset, CR_FD_TTY), &e, PB_TTY);
+}
+
 static int dump_one_pty(int lfd, u32 id, const struct fd_parms *p)
 {
 	TtyFileEntry e = TTY_FILE_ENTRY__INIT;
@@ -686,6 +848,28 @@ static int dump_one_pty(int lfd, u32 id, const struct fd_parms *p)
 		return -1;
 
 	/*
+	 * There might be a control terminal hooked on, if
+	 * so dump it first then continue dumping current.
+	 */
+	if (major(e.rdev) == TTYAUX_MAJOR) {
+		unsigned int sid, prgp;
+
+		sid = tty_get_sid(lfd);
+		prgp = tty_get_prgp(lfd);
+		if (sid < 0 || prgp < 0)
+			return -1;
+
+		if (sid) {
+			if (e.locked)
+				unlock_pty_master(lfd);
+			if (dump_control_pty(pty.index, sid, prgp))
+				return -1;
+			if (e.locked)
+				lock_pty_master(lfd);
+		}
+	}
+
+	/*
 	 * FIXME
 	 *
 	 * Figure out how to fetch data buffered in terminal.
-- 
1.7.7.6



More information about the CRIU mailing list