[CRIU] Re: [PATCH 3/6] tty: Add checkpoint/restore for unix terminals v5

[Pavel Emelyanov]

On 09/11/2012 05:17 PM, Cyrill Gorcunov wrote:
> 
> Usually the PTYs represent a pair of links -- master peer and slave
> peer. Master peer must be opened before slave. Internally, when kernel
> creates master peer it also generates a slave interface in a form of
> /dev/pts/N, where N is that named pty "index". Master/slave connection
> unambiguously identified by this index.
> 
> Still, one master can carry multiple slaves -- for example a user opens
> one master via /dev/ptmx and appropriate /dev/pts/N in sequence.
> The result will be the following
> 
> master
> `- slave 1
> `- slave 2
> 
> both slave will have same master index but different file descriptors.
> Still inside the kernel pty parameters are same for both slaves. Thus
> only one slave parameters should be restored, there is no need to carry
> all parameters for every slave peer we've found.
> 
> Not yet addressed problems:
> 
> - At moment of restore the master peer might be already closed for
>   any reason so to resolve such problem we need to open a fake master
>   peer with proper index and hook a slave on it, then we close
>   master peer.
> 
> - Need to figure out how to deal with ttys which have some
>   data in buffers not yet flushed, at moment this data will
>   be simply lost during c/r
> 
> - Need to restore control terminals
> 
> - Need to fetch tty flags such as exclusive/packet-mode,
>   this can't be done without kernel patching
> 
> [ avagin@:
>    - ideas on contol terminals restore
>    - overall code redesign and simplification
> ]
> 
> v4:
>  - drop redundant pid from dump_chrdev
>  - make sure optional fown is passed on regular ptys
>  - add a comments about zeroifying termios
>  - get rid of redundant empty line in files.c
> 
> v5:
>  - complete rework of tty image format, now we have
>    two files -- tty.img and tty-info.img. The idea
>    behind to reduce data being stored.
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
> CC: Andrey Vagin <avagin at openvz.org>
> ---
>  Makefile              |    1 +
>  cr-dump.c             |    9 +-
>  cr-restore.c          |   10 +
>  files.c               |    7 +
>  image.c               |    2 +
>  include/crtools.h     |    2 +
>  include/image.h       |    2 +
>  include/protobuf.h    |    2 +
>  include/tty.h         |   24 ++
>  protobuf.c            |    3 +
>  protobuf/Makefile     |    1 +
>  protobuf/fdinfo.proto |    1 +
>  protobuf/tty.proto    |   67 ++++
>  tty.c                 |  812 +++++++++++++++++++++++++++++++++++++++++++++++++
>  14 files changed, 938 insertions(+), 5 deletions(-)
>  create mode 100644 include/tty.h
>  create mode 100644 protobuf/tty.proto
>  create mode 100644 tty.c
> 

This all looks very good. One question:

> +static int pty_get_flags(int lfd, int major, int index, TtyInfoEntry *e)
> +{
> +	int slave;
> +
> +	e->locked	= false;
> +	e->exclusive	= false;
> +	e->packet_mode	= false;

> +	/*
> +	 * FIXME
> +	 *
> +	 * PTYs are safe to use packet mode. While there
> +	 * is no way to fetch packet mode settings from
> +	 * the kernel, without it we see echos missing
> +	 * in `screen' application restore.
> +	 */
> +	if (ioctl(master, TIOCPKT, &pktmode) < 0) {
> +		pr_perror("Can't set packed mode");
> +		goto err;
> +	}

Why do we set packet mode in the image to false and treat it as true on restore?