[CRIU] Re: [PATCH 11/14] options: Add "--inherit-sid" option

Cyrill Gorcunov gorcunov at openvz.org
Mon Oct 8 03:20:43 EDT 2012


On Sat, Oct 06, 2012 at 10:58:40AM +0400, Cyrill Gorcunov wrote:
> 
> This option will allow to inherit session
> from the process which runs our tool. This
> is usefull whena subtree of process tree
> is dumped and need to be restored in some
> new environment.

Pavel, since you asked me to make SIDs inheritance by one
option -- please drop patches 11-14 from this series but
review/use the three attached instead.
-------------- next part --------------
>From ea906591e79903242a603a1026a8b11b0ae6d767 Mon Sep 17 00:00:00 2001
From: Cyrill Gorcunov <gorcunov at openvz.org>
Date: Sat, 6 Oct 2012 01:23:21 +0400
Subject: [PATCH 1/3] options: Add "--ext-tty" option

Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
---
 crtools.c         |    5 +++++
 include/crtools.h |    1 +
 2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/crtools.c b/crtools.c
index 714f313..79c276e 100644
--- a/crtools.c
+++ b/crtools.c
@@ -99,6 +99,7 @@ int main(int argc, char *argv[])
 			{ "veth-pair", required_argument, 0, 47},
 			{ "action-script", required_argument, 0, 49},
 			{ LREMAP_PARAM, no_argument, 0, 41},
+			{ "ext-tty", no_argument, 0, 50},
 			{ },
 		};
 
@@ -217,6 +218,9 @@ int main(int argc, char *argv[])
 				list_add(&script->node, &opts.scripts);
 			}
 			break;
+		case 50:
+			opts.ext_tty = true;
+			break;
 		case 'V':
 			pr_msg("Version: %d.%d\n", CRIU_VERSION_MAJOR, CRIU_VERSION_MINOR);
 			return 0;
@@ -312,6 +316,7 @@ usage:
 	pr_msg("			The environment variable CRTOOL_SCRIPT_ACTION contains one of the actions:\n");
 	pr_msg("			* network-lock - lock network in a target network namespace\n");
 	pr_msg("			* network-unlock - unlock network in a target network namespace\n");
+	pr_msg("  --ext-tty             allow to dump and restore external tty session\n");
 
 	pr_msg("\n* Logging:\n");
 	pr_msg("  -o|--log-file [NAME]  log file name (relative path is relative to --images-dir)\n");
diff --git a/include/crtools.h b/include/crtools.h
index 8b0eba2..63695eb 100644
--- a/include/crtools.h
+++ b/include/crtools.h
@@ -93,6 +93,7 @@ struct cr_options {
 	bool			show_pages_content;
 	bool			restore_detach;
 	bool			ext_unix_sk;
+	bool			ext_tty;
 	bool			tcp_established_ok;
 	bool			evasive_devices;
 	bool			link_remap_ok;
-- 
1.7.7.6

-------------- next part --------------
>From 6f5ea599989ad8524f8cdac357755ecb84ee6c73 Mon Sep 17 00:00:00 2001
From: Cyrill Gorcunov <gorcunov at openvz.org>
Date: Sat, 6 Oct 2012 10:39:52 +0400
Subject: [PATCH 2/3] pstree: Migrate SIDs/PGIDs if requested

In case if user asked us to migrate session we need to
walk over all process entries and update SIDs and PGIDs
where needed.

This should be done before we restore the order of process
entities creation.

An example of usage if migrating a task

| # ps
| pid sid comm
|  2   2 bash
|  3   2  `- my_app
|  4   4 bash
|  5   4  `- ps
|
| # crtools dump -t 3 --ext-tty
| # crtools restore -t 3 --ext-tty
| # ps
| pid sid comm
|  4   4 bash
|  5   4  `- crtools
|  3   4      `- my_app

This patch takes into account --ext-tty option to trigger
this feature, which implies that process group of `my_app'
get inherited from crtools itself.

If we ever need to inherit SIDs solely without process
group change -- we simply need to add some new option
for that and set pstree_opts->root_inherit_pgid to false
in prepare_pstree.

The real code to restore external tty connection is addressed
in next patch.

Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
---
 cr-restore.c     |    2 +-
 include/pstree.h |   10 ++++++-
 pstree.c         |   84 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 93 insertions(+), 3 deletions(-)

diff --git a/cr-restore.c b/cr-restore.c
index c961cc6..3f6d219 100644
--- a/cr-restore.c
+++ b/cr-restore.c
@@ -958,7 +958,7 @@ int cr_restore_tasks(pid_t pid, struct cr_options *opts)
 	if (prepare_task_entries() < 0)
 		return -1;
 
-	if (prepare_pstree() < 0)
+	if (prepare_pstree(opts) < 0)
 		return -1;
 
 	if (prepare_pstree_ids() < 0)
diff --git a/include/pstree.h b/include/pstree.h
index 83310c4..8ca1c26 100644
--- a/include/pstree.h
+++ b/include/pstree.h
@@ -25,6 +25,14 @@ struct pid {
 	pid_t virt;
 };
 
+struct pstree_opts {
+	bool			root_inherit_sid;
+	bool			root_inherit_pgid;
+
+	pid_t			new_sid;
+	pid_t			new_pgid;
+};
+
 struct pstree_item {
 	struct pstree_item	*parent;
 	struct list_head	children;	/* list of my children */
@@ -54,7 +62,7 @@ extern struct pstree_item *pstree_item_next(struct pstree_item *item);
 	for (pi = root_item; pi != NULL; pi = pstree_item_next(pi))
 
 extern bool restore_before_setsid(struct pstree_item *child);
-extern int prepare_pstree(void);
+extern int prepare_pstree(struct cr_options *opts);
 extern int prepare_pstree_ids(void);
 
 extern int dump_pstree(struct pstree_item *root_item);
diff --git a/pstree.c b/pstree.c
index 6acf492..ce005d9 100644
--- a/pstree.c
+++ b/pstree.c
@@ -10,6 +10,7 @@
 #include "protobuf/pstree.pb-c.h"
 
 struct pstree_item *root_item;
+static struct pstree_opts *pstree_opts;
 
 void free_pstree(struct pstree_item *root_item)
 {
@@ -107,8 +108,57 @@ err:
 	return ret;
 }
 
+int pstree_inherit(struct pstree_opts *opts)
+{
+	struct pstree_item *pi;
+	pid_t old_sid, old_pgid;
+
+	BUG_ON(!opts);
+
+	if (!opts->root_inherit_sid && !opts->root_inherit_pgid)
+		return 0;
+
+	pr_info("SID/PGID inheritance PID %d (SID %d -> %d PGID %d -> %d)\n",
+		root_item->pid.virt, root_item->sid, opts->new_sid,
+		root_item->pgid, opts->new_pgid);
+
+	old_sid  = root_item->sid;
+	old_pgid = root_item->pgid;
+
+#define SID_PGID_UPDATE(_pi, _old_sid, _old_pgid)		\
+	do {							\
+		if (opts->root_inherit_pgid) {			\
+			if (_pi->pgid == _old_pgid)		\
+				_pi->pgid = opts->new_pgid;	\
+		}						\
+								\
+		if (opts->root_inherit_sid) {			\
+			if (_pi->sid == _old_sid)		\
+				_pi->sid = opts->new_sid;	\
+			if (_pi->pgid == _old_sid)		\
+				_pi->pgid = opts->new_sid;	\
+		}						\
+	} while (0)
+
+	if (opts->root_inherit_pgid)
+		root_item->pgid = opts->new_pgid;
+
+	if (opts->root_inherit_sid) {
+		root_item->sid = opts->new_sid;
+		if (root_item->pgid == old_sid)
+			root_item->pgid = opts->new_sid;
+	}
+
+	for_each_pstree_item(pi)
+		SID_PGID_UPDATE(pi, old_sid, old_pgid);
+
+#undef SID_PGID_UPDATE
+
+	return 0;
+}
+
 static int max_pid = 0;
-int prepare_pstree(void)
+int prepare_pstree(struct cr_options *opts)
 {
 	int ret = 0, i, ps_fd;
 	struct pstree_item *pi, *parent = NULL;
@@ -195,6 +245,31 @@ int prepare_pstree(void)
 		pstree_entry__free_unpacked(e, NULL);
 	}
 
+	if (ret)
+		goto err;
+
+	/*
+	 * SID/PGIDs migration should be done early before we
+	 * do recreate tasks creation order.
+	 */
+	pstree_opts = xzalloc(sizeof(*pstree_opts));
+	if (!pstree_opts)
+		goto err;
+
+	/*
+	 * The reason for two @root_inherit_x options here is
+	 * to save ability to separate SIDs/PGIDs restoration
+	 * in future, if we will ever need it. If in time it
+	 * get known that we never need to restore SIDs only
+	 * we could squash these two options in one.
+	 */
+	pstree_opts->root_inherit_sid	= opts->ext_tty;
+	pstree_opts->root_inherit_pgid	= opts->ext_tty;
+	pstree_opts->new_sid		= getsid(getpid());
+	pstree_opts->new_pgid		= getpgid(getpid());
+
+	ret = pstree_inherit(pstree_opts);
+
 err:
 	close(ps_fd);
 	return ret;
@@ -205,6 +280,8 @@ int prepare_pstree_ids(void)
 	struct pstree_item *item, *child, *helper, *tmp;
 	LIST_HEAD(helpers);
 
+	BUG_ON(!pstree_opts);
+
 	/*
 	 * Some task can be reparented to init. A helper task should be added
 	 * for restoring sid of such tasks. The helper tasks will be exited
@@ -331,6 +408,11 @@ int prepare_pstree_ids(void)
 		if (gleader)
 			continue;
 
+		if (pstree_opts->root_inherit_pgid) {
+			if (pstree_opts->new_pgid == item->pgid)
+				continue;
+		}
+
 		helper = alloc_pstree_item();
 		if (helper == NULL)
 			return -1;
-- 
1.7.7.6

-------------- next part --------------
>From e01739c425de65d25c6a77bbeed2ac91c0ae0f8f Mon Sep 17 00:00:00 2001
From: Cyrill Gorcunov <gorcunov at openvz.org>
Date: Sat, 6 Oct 2012 10:40:33 +0400
Subject: [PATCH 3/3] tty: Migrate tty slave peer connection

In case if we've dumped a slave peer only (say a user dumped
`top' application) we should migrate it on current active
terminal, which barely an own standart stream prepared for us
by file engine.

Note only one external slave peer is allowed simply because
otherwise we can't distinguish which indices should be used
for each of them.

Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
---
 cr-dump.c     |    2 +-
 include/tty.h |    2 +-
 tty.c         |  101 ++++++++++++++++++++++++++++++++++++++++++++------------
 3 files changed, 81 insertions(+), 24 deletions(-)

diff --git a/cr-dump.c b/cr-dump.c
index 1c7a1a2..838840b 100644
--- a/cr-dump.c
+++ b/cr-dump.c
@@ -1629,7 +1629,7 @@ int cr_dump_tasks(pid_t pid, const struct cr_options *opts)
 	if (ret)
 		goto err;
 
-	ret = tty_verify_active_pairs();
+	ret = tty_verify_active_pairs(opts->ext_tty, false);
 	if (ret)
 		goto err;
 
diff --git a/include/tty.h b/include/tty.h
index cb860b1..1b58046 100644
--- a/include/tty.h
+++ b/include/tty.h
@@ -18,7 +18,7 @@ extern int collect_tty(void);
 extern int prepare_shared_tty(void);
 extern void tty_setup_slavery(void);
 
-extern int tty_verify_active_pairs(void);
+extern int tty_verify_active_pairs(bool ext_tty, bool restore);
 
 extern int tty_prep_fds(void);
 extern void tty_fini_fds(void);
diff --git a/tty.c b/tty.c
index d76303c..7c545fc 100644
--- a/tty.c
+++ b/tty.c
@@ -146,11 +146,11 @@ static int tty_get_index(u32 id)
 }
 
 /* Make sure the active pairs do exist */
-int tty_verify_active_pairs(void)
+int tty_verify_active_pairs(bool ext_tty, bool restore)
 {
-	unsigned long i;
+	unsigned long i, j;
 
-	for (i = 0; i < sizeof(tty_active_pairs); i++) {
+	for (i = j = 0; i < sizeof(tty_active_pairs); i++) {
 		i = find_next_bit(tty_active_pairs,
 				  sizeof(tty_active_pairs), i);
 
@@ -163,10 +163,21 @@ int tty_verify_active_pairs(void)
 				continue;
 			}
 
-			pr_err("Found slave peer index %d without "
-				"correspond master peer\n",
-				tty_get_index(i));
-			return -1;
+			if (!ext_tty) {
+				pr_err("Found slave peer index %d without "
+				       "correspond master peer\n",
+				       tty_get_index(i));
+				return -1;
+			}
+
+			if (j++ >= 1) {
+				pr_err("Only one slave external peer "
+				       "is allowed\n");
+				return -1;
+			}
+
+			if (!restore)
+				continue;
 		}
 	}
 
@@ -541,31 +552,67 @@ static int receive_tty(struct tty_info *info)
 	return fd;
 }
 
-static int pty_open_fake_ptmx(struct tty_info *slave)
+static int pty_open_unpaired_slave(struct file_desc *d, struct tty_info *slave)
 {
 	int master = -1, ret = -1, fd = -1;
-	char pts_name[64];
 
-	snprintf(pts_name, sizeof(pts_name), PTS_FMT, slave->tie->pty->index);
+	/*
+	 * We may have 2 cases here:
+	 *
+	 *  - @termios present thus it's external slave peer
+	 *    which should be migrated on current pts
+	 *
+	 *  - no @termios means the peer is dead so we need
+	 *    a fake master peer
+	 */
 
-	master = pty_open_ptmx_index(O_RDONLY, slave->tie->pty->index);
-	if (master < 0) {
-		pr_perror("Can't open fale %x (index %d)",
-			  slave->tfe->id, slave->tie->pty->index);
-		return -1;
-	}
+	if (likely(slave->tie->termios)) {
+		fd = dup(get_service_fd(SELF_STDIN_OFF));
+		if (fd < 0) {
+			pr_perror("Can't dup SELF_STDIN_OFF");
+			return -1;
+		}
+		pr_info("Migrated slave peer %x -> to fd %d\n",
+			slave->tfe->id, fd);
+	} else {
+		char pts_name[64];
 
-	unlock_pty(master);
+		snprintf(pts_name, sizeof(pts_name), PTS_FMT, slave->tie->pty->index);
+
+		master = pty_open_ptmx_index(O_RDONLY, slave->tie->pty->index);
+		if (master < 0) {
+			pr_perror("Can't open fale %x (index %d)",
+				  slave->tfe->id, slave->tie->pty->index);
+			return -1;
+		}
+
+		unlock_pty(master);
+
+		fd = open(pts_name, slave->tfe->flags);
+		if (fd < 0) {
+			pr_perror("Can't open slave %s", pts_name);
+			goto err;
+		}
 
-	fd = open(pts_name, slave->tfe->flags);
-	if (fd < 0) {
-		pr_perror("Can't open slave %s", pts_name);
-		goto err;
 	}
 
 	if (restore_tty_params(fd, slave))
 		goto err;
 
+	/*
+	 * If tty is migrated we need to set its group
+	 * to the parent group, because signals on key
+	 * presses are delivered to a group of terminal.
+	 *
+	 * Note, at this point the group/session should
+	 * be already restored properly thus we can simply
+	 * use syscalls intead of lookup via process tree.
+	 */
+	if (likely(slave->tie->termios)) {
+		if (tty_set_prgp(fd, getpgid(getppid())))
+			goto err;
+	}
+
 	if (pty_open_slaves(slave))
 		goto err;
 
@@ -628,7 +675,7 @@ static int tty_open(struct file_desc *d)
 		return receive_tty(info);
 
 	if (!pty_is_master(info))
-		return pty_open_fake_ptmx(info);
+		return pty_open_unpaired_slave(d, info);
 
 	return pty_open_ptmx(info);
 
@@ -858,6 +905,8 @@ static int collect_one_tty(void *obj, ProtobufCMessage *msg)
 	if (verify_info(info))
 		return -1;
 
+	tty_test_and_set(info->tfe->tty_info_id, tty_active_pairs);
+
 	pr_info("Collected tty ID %#x\n", info->tfe->id);
 
 	list_add(&info->list, &all_ttys);
@@ -877,6 +926,14 @@ int collect_tty(void)
 		ret = collect_image(CR_FD_TTY, PB_TTY,
 				sizeof(struct tty_info),
 				collect_one_tty);
+
+	/*
+	 * Make sure only one external tty present which
+	 * can be migrated.
+	 */
+	if (!ret)
+		ret = tty_verify_active_pairs(true, true);
+
 	return ret;
 }
 
-- 
1.7.7.6



More information about the CRIU mailing list