[CRIU] [PATCH 1/5] tty: Introduce TTY_TYPE

Pavel Emelyanov xemul at parallels.com
Wed Oct 15 02:39:23 PDT 2014


On 10/13/2014 07:51 PM, Cyrill Gorcunov wrote:
> Instead of calling case() with majors all over the
> places lets introduce TTY_TYPE enums and use it
> in future instead.
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
> ---
>  files.c       | 10 ++++----
>  include/tty.h | 16 +++++++++++++
>  tty.c         | 74 ++++++++++++++++++++++++++++++++++++-----------------------
>  3 files changed, 66 insertions(+), 34 deletions(-)
> 
> diff --git a/files.c b/files.c
> index ce8373633b4c..8c0ff1da6e43 100644
> --- a/files.c
> +++ b/files.c
> @@ -273,11 +273,6 @@ static int dump_chrdev(struct fd_parms *p, int lfd, struct cr_img *img)
>  	case MEM_MAJOR:
>  		ops = &regfile_dump_ops;
>  		break;
> -	case TTYAUX_MAJOR:
> -	case UNIX98_PTY_MASTER_MAJOR ... (UNIX98_PTY_MASTER_MAJOR + UNIX98_PTY_MAJOR_COUNT - 1):
> -	case UNIX98_PTY_SLAVE_MAJOR:
> -		ops = &tty_dump_ops;
> -		break;
>  	case MISC_MAJOR:
>  		ops = get_misc_dev_ops(minor(p->stat.st_rdev));
>  		if (ops)
> @@ -286,6 +281,11 @@ static int dump_chrdev(struct fd_parms *p, int lfd, struct cr_img *img)
>  	default: {
>  		char more[32];
>  
> +		if (is_tty(maj, minor(p->stat.st_rdev))) {
> +			ops = &tty_dump_ops;
> +			break;
> +		}
> +
>  		sprintf(more, "%d:%d", maj, minor(p->stat.st_rdev));
>  		return dump_unsupp_fd(p, lfd, img, "chr", more);
>  	}
> diff --git a/include/tty.h b/include/tty.h
> index d2c31e298a34..7181cdd00e6c 100644
> --- a/include/tty.h
> +++ b/include/tty.h
> @@ -6,6 +6,14 @@
>  /* Kernel's limit */
>  #define TERMIOS_NCC	19
>  
> +enum {
> +	TTY_TYPE_UNKNOWN	= 0,
> +	TTY_TYPE_PTM		= 1,
> +	TTY_TYPE_PTS		= 2,
> +
> +	TTY_TYPE_MAX
> +};
> +
>  #define PTMX_PATH	"/dev/ptmx"
>  #ifndef PTMX_MINOR
>  # define PTMX_MINOR 2
> @@ -13,6 +21,14 @@
>  #define PTS_FMT		"/dev/pts/%d"
>  
>  extern const struct fdtype_ops tty_dump_ops;
> +
> +extern int tty_type(int major, int minor);
> +
> +static inline int is_tty(int major, int minor)
> +{
> +	return tty_type(major, minor) != TTY_TYPE_UNKNOWN;
> +}
> +
>  extern int dump_verify_tty_sids(void);
>  extern struct collect_image_info tty_info_cinfo;
>  extern struct collect_image_info tty_cinfo;
> diff --git a/tty.c b/tty.c
> index 31c91a860a20..2742417835de 100644
> --- a/tty.c
> +++ b/tty.c
> @@ -84,7 +84,7 @@ struct tty_info {
>  	TtyInfoEntry			*tie;
>  
>  	struct list_head		sibling;
> -	int				major;
> +	int				type;
>  
>  	bool				create;
>  	bool				inherit;
> @@ -97,7 +97,7 @@ struct tty_dump_info {
>  	pid_t				sid;
>  	pid_t				pgrp;
>  	int				fd;
> -	int				major;
> +	int				type;
>  };
>  
>  static LIST_HEAD(all_tty_info_entries);
> @@ -156,9 +156,9 @@ int prepare_shared_tty(void)
>  		ASSIGN_MEMBER((d),(s), c_line);		\
>  	} while (0)
>  
> -static int tty_gen_id(int major, int index)
> +static int tty_gen_id(int type, int index)
>  {
> -	return (index << 1) + (major == TTYAUX_MAJOR);
> +	return (index << 1) + (type == TTY_TYPE_PTM);
>  }
>  
>  static int tty_get_index(u32 id)
> @@ -199,19 +199,19 @@ int tty_verify_active_pairs(void)
>  	return 0;
>  }
>  
> -static int parse_pty_index(u32 id, int lfd, int major)
> +static int parse_pty_index(u32 id, int lfd, int type)
>  {
>  	int index = -1;
>  
> -	switch (major) {
> -	case TTYAUX_MAJOR:
> +	switch (type) {
> +	case TTY_TYPE_PTM:
>  		if (ioctl(lfd, TIOCGPTN, &index)) {
>  			pr_perror("Can't obtain ptmx index");
>  			return -1;
>  		}
>  		break;
>  
> -	case UNIX98_PTY_SLAVE_MAJOR: {
> +	case TTY_TYPE_PTS: {
>  		char path[PATH_MAX];
>  		char link[32];
>  		int len;
> @@ -378,26 +378,35 @@ static int tty_restore_ctl_terminal(struct file_desc *d, int fd)
>  	return ret;
>  }
>  
> -static char *tty_type(int major)
> +int tty_type(int major, int minor)

Can you place it somewhere so that diff doesn't generate such
a mixed stuff?

>  {
> -	static char *tty_types[] = {
> -		[UNIX98_PTY_SLAVE_MAJOR]	= "pts",
> -		[TTYAUX_MAJOR]			= "ptmx",
> -	};
> -	static char tty_unknown[]		= "unknown";
> -
>  	switch (major) {
> -	case UNIX98_PTY_SLAVE_MAJOR:
>  	case TTYAUX_MAJOR:
> -		return tty_types[major];
> +		if (minor == 0 || minor == 2)

The comparison of minor first appeared in this patch which makes
it not just "introduce the type" but also "fix some bug".

> +			return TTY_TYPE_PTM;
> +		break;
> +	case UNIX98_PTY_MASTER_MAJOR ... (UNIX98_PTY_MASTER_MAJOR + UNIX98_PTY_MAJOR_COUNT - 1):
> +		return TTY_TYPE_PTM;
> +	case UNIX98_PTY_SLAVE_MAJOR:
> +		return TTY_TYPE_PTS;
>  	}
> +	return TTY_TYPE_UNKNOWN;
> +}
>  
> -	return tty_unknown;
> +static char *tty_name(int type)
> +{
> +	switch (type) {
> +	case TTY_TYPE_PTM:
> +		return "ptmx";
> +	case TTY_TYPE_PTS:
> +		return "pts";
> +	}
> +	return "unknown";
>  }
>  
>  static bool tty_is_master(struct tty_info *info)
>  {
> -	return info->major == TTYAUX_MAJOR;
> +	return info->type == TTY_TYPE_PTM;
>  }
>  
>  static bool tty_is_hung(struct tty_info *info)
> @@ -416,7 +425,7 @@ static bool tty_has_active_pair(struct tty_info *info)
>  static void tty_show_pty_info(char *prefix, struct tty_info *info)
>  {
>  	pr_info("%s type %s id %#x index %d (master %d sid %d pgrp %d inherit %d)\n",
> -		prefix, tty_type(info->major), info->tfe->id, info->tie->pty->index,
> +		prefix, tty_name(info->type), info->tfe->id, info->tie->pty->index,
>  		tty_is_master(info), info->tie->sid, info->tie->pgrp, info->inherit);
>  }
>  
> @@ -890,6 +899,13 @@ static int verify_termios(u32 id, TermiosEntry *e)
>  
>  static int verify_info(struct tty_info *info)
>  {
> +	if (info->type != TTY_TYPE_PTM &&
> +	    info->type != TTY_TYPE_PTS) {
> +		pr_err("Unknown type %d master peer %x\n",
> +		       info->type, info->tfe->id);
> +		return -1;
> +	}
> +
>  	/*
>  	 * Master peer must have all parameters present,
>  	 * while slave peer must have either all parameters present
> @@ -973,7 +989,7 @@ static int collect_one_tty(void *obj, ProtobufCMessage *msg)
>  	}
>  
>  	INIT_LIST_HEAD(&info->sibling);
> -	info->major = major(info->tie->rdev);
> +	info->type = tty_type(major(info->tie->rdev), minor(info->tie->rdev));
>  	info->create = tty_is_master(info);
>  	info->inherit = false;
>  
> @@ -1036,7 +1052,7 @@ int dump_verify_tty_sids(void)
>  				if (!opts.shell_job) {
>  					pr_err("Found dangling tty with sid %d pgid %d (%s) on peer fd %d.\n",
>  					       dinfo->sid, dinfo->pgrp,
> -					       tty_type(dinfo->major),
> +					       tty_name(dinfo->type),
>  					       dinfo->fd);
>  					/*
>  					 * First thing people do with criu is dump smth
> @@ -1056,7 +1072,7 @@ int dump_verify_tty_sids(void)
>  	return ret;
>  }
>  
> -static int dump_pty_info(int lfd, u32 id, const struct fd_parms *p, int major, int index)
> +static int dump_pty_info(int lfd, u32 id, const struct fd_parms *p, int type, int index)
>  {
>  	TtyInfoEntry info		= TTY_INFO_ENTRY__INIT;
>  	TermiosEntry termios		= TERMIOS_ENTRY__INIT;
> @@ -1091,7 +1107,7 @@ static int dump_pty_info(int lfd, u32 id, const struct fd_parms *p, int major, i
>  	dinfo->sid		= pti->sid;
>  	dinfo->pgrp		= pti->pgrp;
>  	dinfo->fd		= p->fd;
> -	dinfo->major		= major;
> +	dinfo->type		= type;
>  
>  	list_add_tail(&dinfo->list, &all_ttys);
>  
> @@ -1169,17 +1185,17 @@ out:
>  static int dump_one_pty(int lfd, u32 id, const struct fd_parms *p)
>  {
>  	TtyFileEntry e = TTY_FILE_ENTRY__INIT;
> -	int ret = 0, major, index;
> +	int ret = 0, type, index;
>  
>  	pr_info("Dumping tty %d with id %#x\n", lfd, id);
>  
> -	major = major(p->stat.st_rdev);
> -	index = parse_pty_index(id, lfd, major);
> +	type = tty_type(major(p->stat.st_rdev), minor(p->stat.st_rdev));
> +	index = parse_pty_index(id, lfd, type);
>  	if (index < 0)
>  		return -1;
>  
>  	e.id		= id;
> -	e.tty_info_id	= tty_gen_id(major, index);
> +	e.tty_info_id	= tty_gen_id(type, index);
>  	e.flags		= p->flags;
>  	e.fown		= (FownEntry *)&p->fown;
>  
> @@ -1204,7 +1220,7 @@ static int dump_one_pty(int lfd, u32 id, const struct fd_parms *p)
>  	 */
>  
>  	if (!tty_test_and_set(e.tty_info_id, tty_bitmap))
> -		ret = dump_pty_info(lfd, e.tty_info_id, p, major, index);
> +		ret = dump_pty_info(lfd, e.tty_info_id, p, type, index);
>  
>  	if (!ret)
>  		ret = pb_write_one(img_from_set(glob_imgset, CR_FD_TTY_FILES), &e, PB_TTY_FILE);
> 



More information about the CRIU mailing list