[CRIU] [PATCH 1/5] tty: notify about orphan tty-s via rpc

Pavel Emelyanov xemul at virtuozzo.com
Wed Feb 8 00:47:58 PST 2017


On 02/07/2017 11:43 AM, Andrei Vagin wrote:
> From: Andrei Vagin <avagin at virtuozzo.com>
> 
> Now Docker creates a pty pair from a container devpts to use is as console.
> A slave tty is set as a control tty for the init process and bind-mounted
> into /dev/console. The master tty is handled externelly.
> 
> Now CRIU can handle external resources, but here we have internal resources
> which are used externaly.
> 
> https://github.com/opencontainers/runc/issues/1202
> Signed-off-by: Andrei Vagin <avagin at virtuozzo.com>
> ---
>  criu/action-scripts.c         | 20 ++++++++++++++++++--
>  criu/cr-restore.c             |  1 +
>  criu/cr-service.c             | 27 +++++++++++++++++++++------
>  criu/include/action-scripts.h |  6 +++++-
>  criu/include/cr_options.h     |  1 +
>  criu/include/servicefd.h      |  1 +
>  criu/tty.c                    | 40 +++++++++++++++++++++++++++++++++++-----
>  images/rpc.proto              |  1 +
>  8 files changed, 83 insertions(+), 14 deletions(-)
> 
> diff --git a/criu/action-scripts.c b/criu/action-scripts.c
> index 1a12311..4fa8e28 100644
> --- a/criu/action-scripts.c
> +++ b/criu/action-scripts.c
> @@ -13,6 +13,9 @@
>  #include "pstree.h"
>  #include "common/bug.h"
>  #include "util.h"
> +#include <sys/un.h>
> +#include <sys/socket.h>
> +#include "common/scm.h"
>  
>  static const char *action_names[ACT_MAX] = {
>  	[ ACT_PRE_DUMP ]	= "pre-dump",
> @@ -25,6 +28,7 @@ static const char *action_names[ACT_MAX] = {
>  	[ ACT_POST_SETUP_NS ]	= "post-setup-namespaces",
>  	[ ACT_PRE_RESUME ]	= "pre-resume",
>  	[ ACT_POST_RESUME ]	= "post-resume",
> +	[ ACT_ORPHAN_PTS_MASTER ] = "orphan-pts-master",
>  };
>  
>  struct script {
> @@ -96,6 +100,17 @@ static int run_shell_scripts(const char *action)
>  	return retval;
>  }
>  
> +int rpc_send_fd(enum script_actions act, int fd)
> +{
> +	const char *action = action_names[act];
> +
> +	if (scripts_mode != SCRIPTS_RPC)
> +		return -1;
> +
> +	pr_debug("\tRPC\n");
> +	return send_criu_rpc_script(act, (char *)action, rpc_sk, fd);
> +}
> +
>  int run_scripts(enum script_actions act)
>  {
>  	int ret = 0;
> @@ -108,7 +123,7 @@ int run_scripts(enum script_actions act)
>  
>  	if (scripts_mode == SCRIPTS_RPC) {
>  		pr_debug("\tRPC\n");
> -		ret = send_criu_rpc_script(act, (char *)action, rpc_sk);
> +		ret = send_criu_rpc_script(act, (char *)action, rpc_sk, -1);
>  		goto out;
>  	}
>  
> @@ -146,6 +161,7 @@ int add_rpc_notify(int sk)
>  	BUG_ON(scripts_mode == SCRIPTS_SHELL);
>  	scripts_mode = SCRIPTS_RPC;
>  
> -	rpc_sk = sk;
> +	rpc_sk = install_service_fd(RPC_SK_OFF, sk);
> +
>  	return 0;
>  }
> diff --git a/criu/cr-restore.c b/criu/cr-restore.c
> index ee6b848..9fa9bc3 100644
> --- a/criu/cr-restore.c
> +++ b/criu/cr-restore.c
> @@ -3197,6 +3197,7 @@ static int sigreturn_restore(pid_t pid, struct task_restore_args *task_args, uns
>  	close_proc();
>  	close_service_fd(ROOT_FD_OFF);
>  	close_service_fd(USERNSD_SK);
> +	close_service_fd(RPC_SK_OFF);
>  
>  	__gcov_flush();
>  
> diff --git a/criu/cr-service.c b/criu/cr-service.c
> index 4bac50d..130e2a7 100644
> --- a/criu/cr-service.c
> +++ b/criu/cr-service.c
> @@ -36,6 +36,9 @@
>  #include "irmap.h"
>  #include "kerndat.h"
>  #include "proc_parse.h"
> +#include <sys/un.h>
> +#include <sys/socket.h>
> +#include "common/scm.h"
>  
>  #include "setproctitle.h"
>  
> @@ -84,10 +87,10 @@ err:
>  	return -1;
>  }
>  
> -static int send_criu_msg(int socket_fd, CriuResp *msg)
> +static int send_criu_msg_with_fd(int socket_fd, CriuResp *msg, int fd)
>  {
>  	unsigned char *buf;
> -	int len;
> +	int len, ret;
>  
>  	len = criu_resp__get_packed_size(msg);
>  
> @@ -100,7 +103,11 @@ static int send_criu_msg(int socket_fd, CriuResp *msg)
>  		goto err;
>  	}
>  
> -	if (write(socket_fd, buf, len)  == -1) {
> +	if (fd >= 0) {
> +		ret = send_fds(socket_fd, NULL, 0, &fd, 1, buf, len);
> +	} else
> +		ret = write(socket_fd, buf, len);

I still don't get how the caller is supposed to use this.
Caller sends criu_req(RESTORE) to criu, criu starts restoring and
at some points notices the orphan tty. Then ...
            
> @@ -956,6 +957,32 @@ static int pty_open_unpaired_slave(struct file_desc *d, struct tty_info *slave)
>  	 */
>  
>  	if (likely(slave->inherit)) {
> +		if (opts.orphan_pts_master) {
> +			fake = pty_alloc_fake_master(slave);
> +			if (!fake)
> +				goto err;
> +			master = pty_open_ptmx_index(&fake->d, slave, O_RDWR);
> +			if (master < 0) {
> +				pr_err("Can't open master pty %x (index %d)\n",
> +					  slave->tfe->id, slave->tie->pty->index);
> +				goto err;
> +			}
> +
> +			unlock_pty(master);
> +
> +			if (opts.orphan_pts_master &&
> +			    rpc_send_fd(ACT_ORPHAN_PTS_MASTER, master) == 0) {

It just sends the fd to the caller. Caller's recvmsg wakes up and ... sees no
message in the queue, only the new file descriptor. How is he supposed to tell
action-script notification (which he expects with recvmsg) from the orphan tty
fd received?

> +
> +				fd = open_tty_reg(slave->reg_d, slave->tfe->flags);
> +				if (fd < 0) {
> +					pr_err("Can't open slave pty %s\n", path_from_reg(slave->reg_d));
> +					goto err;
> +				}
> +
> +				goto out;
> +			}
> +		}
> +
>  		if (!stdin_isatty) {
>  			pr_err("Don't have tty to inherit session from, aborting\n");
>  			return -1;
> @@ -990,6 +1017,7 @@ static int pty_open_unpaired_slave(struct file_desc *d, struct tty_info *slave)
>  
>  	}
>  
> +out:
>  	if (restore_tty_params(fd, slave))
>  		goto err;
>  
> @@ -1002,7 +1030,7 @@ static int pty_open_unpaired_slave(struct file_desc *d, struct tty_info *slave)
>  	 * be already restored properly thus we can simply
>  	 * use syscalls instead of lookup via process tree.
>  	 */
> -	if (likely(slave->inherit)) {
> +	if (slave->inherit && opts.shell_job) {
>  		/*
>  		 * The restoration procedure only works if we're
>  		 * migrating not a session leader, otherwise it's
> @@ -1241,8 +1269,10 @@ static int tty_find_restoring_task(struct tty_info *info)
>  		if (!tty_is_master(info)) {
>  			if (tty_has_active_pair(info))
>  				return 0;
> -			else
> +			else if (!opts.orphan_pts_master)
>  				goto shell_job;
> +			else
> +				info->inherit = true;
>  		}
>  
>  		/*
> @@ -1366,7 +1396,7 @@ static int tty_setup_slavery(void * unused)
>  		    info->driver->type == TTY_TYPE__CTTY)
>  			continue;
>  
> -		if (!tty_is_master(info))
> +		if (!tty_is_master(info) && info->link)
>  			continue;
>  
>  		info->ctl_tty = info;
> diff --git a/images/rpc.proto b/images/rpc.proto
> index 4811738..ddc28db 100644
> --- a/images/rpc.proto
> +++ b/images/rpc.proto
> @@ -109,6 +109,7 @@ message criu_opts {
>  	optional bool			tcp_skip_in_flight	= 46;
>  	optional bool			weak_sysctls		= 47;
>  	optional bool			lazy_pages		= 48;
> +	optional bool			orphan_pts_master	= 49;
>  }
>  
>  message criu_dump_resp {
> 



More information about the CRIU mailing list