[v2,03/14] files: Move CTL_TTY_OFF fixup to generic file engine

Submitted by Kirill Tkhai on Dec. 28, 2017, 1:09 p.m.

Details

Message ID 151446655312.28132.9007655429599304884.stgit@localhost.localdomain
State New
Series "Introduce custom per-task service fds placement"
Headers show

Commit Message

Kirill Tkhai Dec. 28, 2017, 1:09 p.m.
There are two problems. The first is CTL_TTY_OFF occupies
one of the biggest available fds in the system. It's a number
near service_fd_rlim_cur. Next patches want to allocate
service fds lower, than service_fd_rlim_cur, and they want
to know max used fd from file fles after the image reading.

But since one of fds is already set very big (CTL_TTY_OFF)
on a stage of collection fles, the only availabe service
fds are near service_fd_rlim_cur. It's vicious circle,
and the only way is to change ctl tty fd allocation way.

The second problem is ctl tty is ugly out of generic file
engine fixup (see open_fd()). This is made because ctl tty
is the only slave fle, which needs additional actions
(see tty_restore_ctl_terminal()). Another file types just
receive their slave fle, and do not do anything else.

This patch moves ctl tty to generic engine and solves all
the above problems. To do that, we implement new CTL_TTY
file type, which open method waits till slave tty is received
and then calls tty_restore_ctl_terminal() for that. It fits
to generic engine well, and allocates fd via find_unused_fd(),
and do not polute file table by big fd numbers.

Next patch will kill currently unneed CTL_TTY leftovers
and will remove CTL_TTY_OFF service fd from criu.

Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
---
 criu/files.c        |   13 +-----
 criu/include/tty.h  |    2 -
 criu/tty.c          |  111 ++++++++++++++++++++++++++++++++++++++++++++++-----
 images/fdinfo.proto |    1 
 4 files changed, 104 insertions(+), 23 deletions(-)

Patch hide | download patch | download mbox

diff --git a/criu/files.c b/criu/files.c
index 5aaaef104..705fecf5f 100644
--- a/criu/files.c
+++ b/criu/files.c
@@ -1139,7 +1139,7 @@  static int open_fd(struct fdinfo_list_entry *fle)
 		ret = receive_fd(fle);
 		if (ret != 0)
 			return ret;
-		goto fixup_ctty;
+		goto out;
 	}
 
 	/*
@@ -1160,16 +1160,9 @@  static int open_fd(struct fdinfo_list_entry *fle)
 		if (setup_and_serve_out(fle, new_fd) < 0)
 			return -1;
 	}
-fixup_ctty:
-	if (ret == 0) {
-		if (fle->fe->fd == get_service_fd(CTL_TTY_OFF)) {
-			ret = tty_restore_ctl_terminal(fle->desc, fle->fe->fd);
-			if (ret == -1)
-				return ret;
-		}
-
+out:
+	if (ret == 0)
 		fle->stage = FLE_RESTORED;
-	}
 	return ret;
 }
 
diff --git a/criu/include/tty.h b/criu/include/tty.h
index 5550a72c4..25fa1c42c 100644
--- a/criu/include/tty.h
+++ b/criu/include/tty.h
@@ -34,8 +34,6 @@  extern int devpts_restore(struct mount_info *pm);
 extern int tty_prep_fds(void);
 extern void tty_fini_fds(void);
 
-extern int tty_restore_ctl_terminal(struct file_desc *d, int fd);
-
 extern int devpts_check_bindmount(struct mount_info *m);
 
 #define OPT_SHELL_JOB	"shell-job"
diff --git a/criu/tty.c b/criu/tty.c
index f06c6cc60..9ef60d224 100644
--- a/criu/tty.c
+++ b/criu/tty.c
@@ -686,7 +686,7 @@  static int tty_set_prgp(int fd, int group)
 	return 0;
 }
 
-int tty_restore_ctl_terminal(struct file_desc *d, int fd)
+static int tty_restore_ctl_terminal(struct file_desc *d)
 {
 	struct tty_info *info = container_of(d, struct tty_info, d);
 	struct tty_driver *driver = info->driver;
@@ -694,8 +694,6 @@  int tty_restore_ctl_terminal(struct file_desc *d, int fd)
 	struct file_desc *slave_d;
 	int slave = -1, ret = -1, index = -1;
 
-	BUG_ON(!is_service_fd(fd, CTL_TTY_OFF));
-
 	if (driver->type == TTY_TYPE__EXT_TTY) {
 		slave = -1;
 		if (!inherited_fd(&info->d, &slave) && slave < 0)
@@ -1229,23 +1227,18 @@  static struct pstree_item *find_first_sid(int sid)
 	return NULL;
 }
 
-static int prepare_ctl_tty(struct pstree_item *item, u32 ctl_tty_id)
+static int add_fake_fle(struct pstree_item *item, u32 desc_id)
 {
 	FdinfoEntry *e;
 
-	if (!ctl_tty_id)
-		return 0;
-
-	pr_info("Requesting for ctl tty %#x into service fd\n", ctl_tty_id);
-
 	e = xmalloc(sizeof(*e));
 	if (!e)
 		return -1;
 
 	fdinfo_entry__init(e);
 
-	e->id		= ctl_tty_id;
-	e->fd		= reserve_service_fd(CTL_TTY_OFF);
+	e->id		= desc_id;
+	e->fd		= find_unused_fd(item, -1);
 	e->type		= FD_TYPES__TTY;
 
 	if (collect_fd(vpid(item), e, rsti(item), true)) {
@@ -1253,7 +1246,103 @@  static int prepare_ctl_tty(struct pstree_item *item, u32 ctl_tty_id)
 		return -1;
 	}
 
+	return e->fd;
+}
+
+struct ctl_tty {
+	struct file_desc desc;
+	struct fdinfo_list_entry *real_tty;
+};
+
+static int ctl_tty_open(struct file_desc *d, int *new_fd)
+{
+	struct fdinfo_list_entry *fle;
+	int ret;
+
+	fle = container_of(d, struct ctl_tty, desc)->real_tty;
+	if (fle->stage != FLE_RESTORED)
+		return 1;
+
+	ret = tty_restore_ctl_terminal(fle->desc);
+	if (!ret) {
+		/*
+		 * Generic engine expects we return a new_fd.
+		 * Return this one just to return something.
+		 */
+		*new_fd = dup(fle->fe->fd);
+		if (*new_fd < 0) {
+			pr_perror("dup() failed");
+			ret = -1;
+		} else
+			ret = 0;
+	}
+	return ret;
+}
+
+/*
+ * This is a fake type to handle ctl tty. The problem
+ * is sometimes we need to do tty_set_sid() from slave
+ * fle, while generic file engine allows to call open
+ * method for file masters only. So, this type allows
+ * to add fake masters, which will call open for slave
+ * fles of type FD_TYPES__TTY indirectly.
+ */
+static struct file_desc_ops ctl_tty_desc_ops = {
+	.type		= FD_TYPES__CTL_TTY,
+	.open		= ctl_tty_open,
+};
+
+static int prepare_ctl_tty(struct pstree_item *item, u32 ctl_tty_id)
+{
+	struct fdinfo_list_entry *fle;
+	struct ctl_tty *ctl_tty;
+	FdinfoEntry *e;
+	int fd;
+
+	if (!ctl_tty_id)
+		return 0;
+
+	pr_info("Requesting for ctl tty %#x into service fd\n", ctl_tty_id);
+
+	/* Add a fake fle to make generic engine deliver real tty desc to task */
+	fd = add_fake_fle(item, ctl_tty_id);
+	if (fd < 0)
+		return -1;
+
+	fle = find_used_fd(item, fd);
+	BUG_ON(!fle);
+	/*
+	 * Add a fake ctl_tty depending on the above fake fle, which will
+	 * actually restore the session.
+	 */
+	ctl_tty	= xmalloc(sizeof(*ctl_tty));
+	e	= xmalloc(sizeof(*e));
+
+	if (!ctl_tty || !e)
+		goto err;
+
+	ctl_tty->real_tty = fle;
+
+	/*
+	 * Use the same ctl_tty_id id for ctl_tty as it's unique among
+	 * FD_TYPES__CTL_TTY (as it's unique for FD_TYPES__TTY type).
+	 */
+	file_desc_add(&ctl_tty->desc, ctl_tty_id, &ctl_tty_desc_ops);
+
+	fdinfo_entry__init(e);
+
+	e->id		= ctl_tty_id;
+	e->fd		= find_unused_fd(item, -1);
+	e->type		= FD_TYPES__CTL_TTY;
+
+	if (collect_fd(vpid(item), e, rsti(item), true))
+		goto err;
+
 	return 0;
+err:
+	xfree(ctl_tty);
+	xfree(e);
+	return -1;
 }
 
 static int tty_find_restoring_task(struct tty_info *info)
diff --git a/images/fdinfo.proto b/images/fdinfo.proto
index 3d4e50701..ed82ceffe 100644
--- a/images/fdinfo.proto
+++ b/images/fdinfo.proto
@@ -38,6 +38,7 @@  enum fd_types {
 	TIMERFD		= 17;
 
 	/* Any number above the real used. Not stored to image */
+	CTL_TTY		= 65534;
 	AUTOFS_PIPE	= 65535;
 }