[3/7] tty: Save mount ids for terminals into image

Submitted by Cyrill Gorcunov on Feb. 2, 2017, 8:02 a.m.

Details

Message ID 1486022529-1901-4-git-send-email-gorcunov@openvz.org
State New
Series "tty: Add support for multiple devpts instances"
Headers show

Commit Message

Cyrill Gorcunov Feb. 2, 2017, 8:02 a.m.
Since we're gonna to handle several devpts instance
each terminal need a second key to separate same
named pairs in different mounts. For this sake
each tty assigned with mount id.

Because one tty_info_entry may be mapped from
several different tty_file_entry (say someone
opens /dev/pts/X several times) we need @mnt_id
to be in both image entries.

Note to restore them properly we need per mount
bitmaps to track pairs and indices, which will be
addressed in next patch.

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
---
 criu/tty.c       | 52 +++++++++++++++++++++++++++++++++++++++++++++++++---
 images/tty.proto |  6 ++++++
 2 files changed, 55 insertions(+), 3 deletions(-)

Patch hide | download patch | download mbox

diff --git a/criu/tty.c b/criu/tty.c
index 45c9293fe66d..2c0c56c57515 100644
--- a/criu/tty.c
+++ b/criu/tty.c
@@ -28,6 +28,7 @@ 
 #include "files-reg.h"
 #include "namespaces.h"
 #include "external.h"
+#include "mount.h"
 
 #include "protobuf.h"
 #include "util.h"
@@ -107,6 +108,7 @@  struct tty_dump_info {
 	struct list_head		list;
 
 	u32				id;
+	int				mnt_id;
 	pid_t				sid;
 	pid_t				pgrp;
 	int				fd;
@@ -1479,6 +1481,11 @@  static int collect_one_tty_info_entry(void *obj, ProtobufCMessage *msg, struct c
 
 	info->tie = pb_msg(msg, TtyInfoEntry);
 
+	if (!info->tie->has_mnt_id) {
+		info->tie->has_mnt_id = true;
+		info->tie->mnt_id = 0;
+	}
+
 	switch (info->tie->type) {
 	case TTY_TYPE__PTY:
 		if (!info->tie->pty) {
@@ -1523,6 +1530,11 @@  static int collect_one_tty(void *obj, ProtobufCMessage *msg, struct cr_img *i)
 
 	info->tfe = pb_msg(msg, TtyFileEntry);
 
+	if (!info->tfe->has_mnt_id) {
+		info->tfe->has_mnt_id = true;
+		info->tfe->mnt_id = 0;
+	}
+
 	info->tie = lookup_tty_info_entry(info->tfe->tty_info_id);
 	if (!info->tie) {
 		pr_err("No tty-info-id %#x found on id %#x\n",
@@ -1611,6 +1623,11 @@  static int collect_one_tty_data(void *obj, ProtobufCMessage *msg, struct cr_img
 	struct tty_info *info;
 
 	tdo->tde = pb_msg(msg, TtyDataEntry);
+	if (!tdo->tde->has_mnt_id) {
+		tdo->tde->has_mnt_id = true;
+		tdo->tde->mnt_id = 0;
+	}
+
 	pr_debug("Collected data for id %#x (size %zu bytes)\n",
 		 tdo->tde->tty_id, (size_t)tdo->tde->data.len);
 
@@ -1682,7 +1699,8 @@  int dump_verify_tty_sids(void)
 	return ret;
 }
 
-static int dump_tty_info(int lfd, u32 id, const struct fd_parms *p, struct tty_driver *driver, int index)
+static int dump_tty_info(int lfd, u32 id, const struct fd_parms *p, int mnt_id,
+			 struct tty_driver *driver, int index)
 {
 	TtyInfoEntry info		= TTY_INFO_ENTRY__INIT;
 	TermiosEntry termios		= TERMIOS_ENTRY__INIT;
@@ -1719,6 +1737,7 @@  static int dump_tty_info(int lfd, u32 id, const struct fd_parms *p, struct tty_d
 	dinfo->fd		= p->fd;
 	dinfo->driver		= driver;
 	dinfo->flags		= p->flags;
+	dinfo->mnt_id		= mnt_id;
 
 	if (is_pty(driver)) {
 		dinfo->lfd = dup(lfd);
@@ -1750,6 +1769,9 @@  static int dump_tty_info(int lfd, u32 id, const struct fd_parms *p, struct tty_d
 	info.has_gid		= true;
 	info.gid		= userns_gid(p->stat.st_gid);
 
+	info.has_mnt_id		= true;
+	info.mnt_id		= mnt_id;
+
 	info.type = driver->type;
 	if (info.type == TTY_TYPE__PTY) {
 		info.pty	= &pty;
@@ -1818,7 +1840,7 @@  out:
 static int dump_one_tty(int lfd, u32 id, const struct fd_parms *p)
 {
 	TtyFileEntry e = TTY_FILE_ENTRY__INIT;
-	int ret = 0, index = -1;
+	int ret = 0, index = -1, mnt_id;
 	struct tty_driver *driver;
 
 	pr_info("Dumping tty %d with id %#x\n", lfd, id);
@@ -1842,6 +1864,28 @@  static int dump_one_tty(int lfd, u32 id, const struct fd_parms *p)
 	e.flags		= p->flags;
 	e.fown		= (FownEntry *)&p->fown;
 
+	if (is_pty(driver)) {
+		mnt_id = mount_resolve_devpts_mnt_id(p->stat.st_dev);
+		if (mnt_id < 0) {
+			pr_info("Can't obtain mnt_id on tty %d id %#x\n", lfd, id);
+			return -1;
+		}
+	} else {
+		/*
+		 * Strictly speaking at moment there is
+		 * no strong need to keep @mnt_id for non-PTY
+		 * terminals, but having them here is suitable
+		 * for debug purpose and might be needed in
+		 * future when these devices will be virtualized.
+		 * After all having unified scheme for all terminals
+		 * is a big win.
+		 */
+		mnt_id = p->mnt_id;
+	}
+
+	e.has_mnt_id	= true;
+	e.mnt_id	= mnt_id;
+
 	/*
 	 * FIXME
 	 *
@@ -1863,7 +1907,7 @@  static int dump_one_tty(int lfd, u32 id, const struct fd_parms *p)
 	 */
 
 	if (!tty_test_and_set(e.tty_info_id, tty_bitmap))
-		ret = dump_tty_info(lfd, e.tty_info_id, p, driver, index);
+		ret = dump_tty_info(lfd, e.tty_info_id, p, mnt_id, driver, index);
 
 	if (!ret)
 		ret = pb_write_one(img_from_set(glob_imgset, CR_FD_TTY_FILES), &e, PB_TTY_FILE);
@@ -1963,6 +2007,8 @@  static int tty_do_dump_queued_data(struct tty_dump_info *dinfo)
 		e.tty_id	= dinfo->id;
 		e.data.data	= (void *)buf;
 		e.data.len	= off;
+		e.has_mnt_id	= true;
+		e.mnt_id	= dinfo->mnt_id;
 
 		ret = pb_write_one(img_from_set(glob_imgset, CR_FD_TTY_DATA),
 				   &e, PB_TTY_DATA);
diff --git a/images/tty.proto b/images/tty.proto
index 739a4ffedfff..4b36ef7c005e 100644
--- a/images/tty.proto
+++ b/images/tty.proto
@@ -39,6 +39,8 @@  enum TtyType {
 message tty_data_entry {
 	required uint32			tty_id		= 1;
 	required bytes			data		= 2;
+
+	optional sint32			mnt_id		= 3 [default = 0];
 }
 
 message tty_info_entry {
@@ -73,6 +75,8 @@  message tty_info_entry {
 
 	optional uint32			uid		= 14;
 	optional uint32			gid		= 15;
+
+	optional sint32			mnt_id		= 16 [default = 0];
 };
 
 message tty_file_entry {
@@ -81,4 +85,6 @@  message tty_file_entry {
 
 	required uint32			flags		= 3 [(criu).hex = true];
 	required fown_entry		fown		= 4;
+
+	optional sint32			mnt_id		= 5 [default = 0];
 }

Comments

Pavel Emelianov Feb. 8, 2017, 8:35 a.m.
On 02/02/2017 11:02 AM, Cyrill Gorcunov wrote:
> Since we're gonna to handle several devpts instance
> each terminal need a second key to separate same
> named pairs in different mounts. For this sake
> each tty assigned with mount id.
> 
> Because one tty_info_entry may be mapped from
> several different tty_file_entry (say someone
> opens /dev/pts/X several times) we need @mnt_id
> to be in both image entries.
> 
> Note to restore them properly we need per mount
> bitmaps to track pairs and indices, which will be
> addressed in next patch.
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
> ---
>  criu/tty.c       | 52 +++++++++++++++++++++++++++++++++++++++++++++++++---
>  images/tty.proto |  6 ++++++
>  2 files changed, 55 insertions(+), 3 deletions(-)
> 
> diff --git a/criu/tty.c b/criu/tty.c
> index 45c9293fe66d..2c0c56c57515 100644
> --- a/criu/tty.c
> +++ b/criu/tty.c
> @@ -28,6 +28,7 @@
>  #include "files-reg.h"
>  #include "namespaces.h"
>  #include "external.h"
> +#include "mount.h"
>  
>  #include "protobuf.h"
>  #include "util.h"
> @@ -107,6 +108,7 @@ struct tty_dump_info {
>  	struct list_head		list;
>  
>  	u32				id;
> +	int				mnt_id;
>  	pid_t				sid;
>  	pid_t				pgrp;
>  	int				fd;
> @@ -1479,6 +1481,11 @@ static int collect_one_tty_info_entry(void *obj, ProtobufCMessage *msg, struct c
>  
>  	info->tie = pb_msg(msg, TtyInfoEntry);
>  
> +	if (!info->tie->has_mnt_id) {
> +		info->tie->has_mnt_id = true;
> +		info->tie->mnt_id = 0;
> +	}

Would you explain this?

> +
>  	switch (info->tie->type) {
>  	case TTY_TYPE__PTY:
>  		if (!info->tie->pty) {
> @@ -1523,6 +1530,11 @@ static int collect_one_tty(void *obj, ProtobufCMessage *msg, struct cr_img *i)
>  
>  	info->tfe = pb_msg(msg, TtyFileEntry);
>  
> +	if (!info->tfe->has_mnt_id) {
> +		info->tfe->has_mnt_id = true;
> +		info->tfe->mnt_id = 0;
> +	}

And this.

> +
>  	info->tie = lookup_tty_info_entry(info->tfe->tty_info_id);
>  	if (!info->tie) {
>  		pr_err("No tty-info-id %#x found on id %#x\n",
> @@ -1611,6 +1623,11 @@ static int collect_one_tty_data(void *obj, ProtobufCMessage *msg, struct cr_img
>  	struct tty_info *info;
>  
>  	tdo->tde = pb_msg(msg, TtyDataEntry);
> +	if (!tdo->tde->has_mnt_id) {
> +		tdo->tde->has_mnt_id = true;
> +		tdo->tde->mnt_id = 0;
> +	}

And this. All mnt_id-s are zeroed by pb decoder, aren't they?

> +
>  	pr_debug("Collected data for id %#x (size %zu bytes)\n",
>  		 tdo->tde->tty_id, (size_t)tdo->tde->data.len);
>  
> @@ -1682,7 +1699,8 @@ int dump_verify_tty_sids(void)
>  	return ret;
>  }
>  
> -static int dump_tty_info(int lfd, u32 id, const struct fd_parms *p, struct tty_driver *driver, int index)
> +static int dump_tty_info(int lfd, u32 id, const struct fd_parms *p, int mnt_id,
> +			 struct tty_driver *driver, int index)
>  {
>  	TtyInfoEntry info		= TTY_INFO_ENTRY__INIT;
>  	TermiosEntry termios		= TERMIOS_ENTRY__INIT;
> @@ -1719,6 +1737,7 @@ static int dump_tty_info(int lfd, u32 id, const struct fd_parms *p, struct tty_d
>  	dinfo->fd		= p->fd;
>  	dinfo->driver		= driver;
>  	dinfo->flags		= p->flags;
> +	dinfo->mnt_id		= mnt_id;
>  
>  	if (is_pty(driver)) {
>  		dinfo->lfd = dup(lfd);
> @@ -1750,6 +1769,9 @@ static int dump_tty_info(int lfd, u32 id, const struct fd_parms *p, struct tty_d
>  	info.has_gid		= true;
>  	info.gid		= userns_gid(p->stat.st_gid);
>  
> +	info.has_mnt_id		= true;
> +	info.mnt_id		= mnt_id;
> +
>  	info.type = driver->type;
>  	if (info.type == TTY_TYPE__PTY) {
>  		info.pty	= &pty;
> @@ -1818,7 +1840,7 @@ out:
>  static int dump_one_tty(int lfd, u32 id, const struct fd_parms *p)
>  {
>  	TtyFileEntry e = TTY_FILE_ENTRY__INIT;
> -	int ret = 0, index = -1;
> +	int ret = 0, index = -1, mnt_id;
>  	struct tty_driver *driver;
>  
>  	pr_info("Dumping tty %d with id %#x\n", lfd, id);
> @@ -1842,6 +1864,28 @@ static int dump_one_tty(int lfd, u32 id, const struct fd_parms *p)
>  	e.flags		= p->flags;
>  	e.fown		= (FownEntry *)&p->fown;
>  
> +	if (is_pty(driver)) {
> +		mnt_id = mount_resolve_devpts_mnt_id(p->stat.st_dev);
> +		if (mnt_id < 0) {
> +			pr_info("Can't obtain mnt_id on tty %d id %#x\n", lfd, id);
> +			return -1;
> +		}
> +	} else {
> +		/*
> +		 * Strictly speaking at moment there is
> +		 * no strong need to keep @mnt_id for non-PTY
> +		 * terminals, but having them here is suitable
> +		 * for debug purpose and might be needed in
> +		 * future when these devices will be virtualized.
> +		 * After all having unified scheme for all terminals
> +		 * is a big win.
> +		 */
> +		mnt_id = p->mnt_id;
> +	}
> +
> +	e.has_mnt_id	= true;
> +	e.mnt_id	= mnt_id;
> +
>  	/*
>  	 * FIXME
>  	 *
> @@ -1863,7 +1907,7 @@ static int dump_one_tty(int lfd, u32 id, const struct fd_parms *p)
>  	 */
>  
>  	if (!tty_test_and_set(e.tty_info_id, tty_bitmap))
> -		ret = dump_tty_info(lfd, e.tty_info_id, p, driver, index);
> +		ret = dump_tty_info(lfd, e.tty_info_id, p, mnt_id, driver, index);
>  
>  	if (!ret)
>  		ret = pb_write_one(img_from_set(glob_imgset, CR_FD_TTY_FILES), &e, PB_TTY_FILE);
> @@ -1963,6 +2007,8 @@ static int tty_do_dump_queued_data(struct tty_dump_info *dinfo)
>  		e.tty_id	= dinfo->id;
>  		e.data.data	= (void *)buf;
>  		e.data.len	= off;
> +		e.has_mnt_id	= true;
> +		e.mnt_id	= dinfo->mnt_id;
>  
>  		ret = pb_write_one(img_from_set(glob_imgset, CR_FD_TTY_DATA),
>  				   &e, PB_TTY_DATA);
> diff --git a/images/tty.proto b/images/tty.proto
> index 739a4ffedfff..4b36ef7c005e 100644
> --- a/images/tty.proto
> +++ b/images/tty.proto
> @@ -39,6 +39,8 @@ enum TtyType {
>  message tty_data_entry {
>  	required uint32			tty_id		= 1;
>  	required bytes			data		= 2;
> +
> +	optional sint32			mnt_id		= 3 [default = 0];

Default is zero w/o explicit mark.
The tty_id references tty_info_entry which already has mnt_id.

>  }
>  
>  message tty_info_entry {
> @@ -73,6 +75,8 @@ message tty_info_entry {
>  
>  	optional uint32			uid		= 14;
>  	optional uint32			gid		= 15;
> +
> +	optional sint32			mnt_id		= 16 [default = 0];
>  };
>  
>  message tty_file_entry {
> @@ -81,4 +85,6 @@ message tty_file_entry {
>  
>  	required uint32			flags		= 3 [(criu).hex = true];
>  	required fown_entry		fown		= 4;
> +
> +	optional sint32			mnt_id		= 5 [default = 0];

tty_file_entry.info_id references tty_info_entry with mnt_id on board.

>  }
>
Cyrill Gorcunov Feb. 8, 2017, 3:54 p.m.
On Wed, Feb 08, 2017 at 11:35:25AM +0300, Pavel Emelyanov wrote:
> > @@ -1479,6 +1481,11 @@ static int collect_one_tty_info_entry(void *obj, ProtobufCMessage *msg, struct c
> >  
> >  	info->tie = pb_msg(msg, TtyInfoEntry);
> >  
> > +	if (!info->tie->has_mnt_id) {
> > +		info->tie->has_mnt_id = true;
> > +		info->tie->mnt_id = 0;
> > +	}
> 
> Would you explain this?

Not sure what exactly to explain here, explicit zeroifying?
Yes, decoder should zero them (and in most case it does) but
just to be on safe side I did it explicitly.

> > @@ -39,6 +39,8 @@ enum TtyType {
> >  message tty_data_entry {
> >  	required uint32			tty_id		= 1;
> >  	required bytes			data		= 2;
> > +
> > +	optional sint32			mnt_id		= 3 [default = 0];
> 
> Default is zero w/o explicit mark.
> The tty_id references tty_info_entry which already has mnt_id.

tty_id is not longer unique, with multiple devpts each tty and
related resource is addressed with two keys: tty_id + mnt_id,
so every resource (tty-info, tty-file, tty-data) must carry
own mnt_id, or we need rework tty code to make tty-id to
be unique but in this case readin tty[-image]|[-data] will
be a way more harder with crit help, believe me, i tried.

	Cyrill