[CRIU] [PATCH 4/4] cr-service: use inherit-fd keys to ask for FD
Andrei Vagin
avagin at virtuozzo.com
Tue Aug 7 00:01:05 MSK 2018
On Mon, Aug 06, 2018 at 04:55:00PM +0000, Adrian Reber wrote:
> From: Adrian Reber <areber at redhat.com>
>
> Using inherit-fd in RPC mode uses the notify mechanism to ask the RPC
> client for the real FD.
>
> The basic workflow of inherit-fd in RPC mode is:
>
> * RPC clients fills in inherit-fd field:
> * inheritFd.Key = proto.String(path.Base(nsPath))
> * inheritFd.Fd is irrelevant as it will be transmitted later
> * CRIU receives RPC messages and calls inherit_fd_add_rpc()
> * this transmits the inheritFd.Key back to the RPC client
> * RPC client uses unix domain socket to send the real FD
> to CRIU
>
> Unfortunately this is more complicated as runc already uses
> inheritFd.Fd to send the file descriptors for 0, 1, 2. As those are
> always stdin, stdout, stderr is just worked even if the transmitted
> values via RPC were not actual FDs but only numbers.
>
> To handle this, CRIU still accepts inherit-fds directly as FDs if it is
> 0, 1, 2. This is a compatibility hack to avoid breaking runc.
>
> Signed-off-by: Adrian Reber <areber at redhat.com>
> ---
> criu/action-scripts.c | 10 ++++++---
> criu/cr-service.c | 38 ++++++++++++++++++++++++++++++++---
> criu/files.c | 10 +++++++++
> criu/include/action-scripts.h | 4 ++--
> criu/include/files.h | 1 +
> criu/tty.c | 2 +-
> 6 files changed, 56 insertions(+), 9 deletions(-)
>
> diff --git a/criu/action-scripts.c b/criu/action-scripts.c
> index 4e9eb65cf..1a7523527 100644
> --- a/criu/action-scripts.c
> +++ b/criu/action-scripts.c
> @@ -100,7 +100,11 @@ static int run_shell_scripts(const char *action)
> return retval;
> }
>
> -int rpc_send_fd(enum script_actions act, int fd)
> +/*
> + * The name of this function is misleading. Right now it only
> + * sends an FD in one of three possible use cases.
> + */
> +int rpc_send_fd(enum script_actions act, int fd, char *key)
> {
> const char *action = action_names[act];
> int rpc_sk;
> @@ -113,7 +117,7 @@ int rpc_send_fd(enum script_actions act, int fd)
> return -1;
>
> pr_debug("\tRPC\n");
> - return send_criu_rpc_script(act, (char *)action, rpc_sk, fd);
> + return send_criu_rpc_script(act, (char *)action, rpc_sk, fd, key);
> }
>
> int run_scripts(enum script_actions act)
> @@ -127,7 +131,7 @@ int run_scripts(enum script_actions act)
> return 0;
>
> if (scripts_mode == SCRIPTS_RPC) {
> - ret = rpc_send_fd(act, -1);
> + ret = rpc_send_fd(act, -1, NULL);
> goto out;
> }
>
> diff --git a/criu/cr-service.c b/criu/cr-service.c
> index 643aba9cf..45d8b3f44 100644
> --- a/criu/cr-service.c
> +++ b/criu/cr-service.c
> @@ -188,9 +188,10 @@ int send_criu_restore_resp(int socket_fd, bool success, int pid)
> return send_criu_msg(socket_fd, &msg);
> }
>
> -int send_criu_rpc_script(enum script_actions act, char *name, int sk, int fd)
> +int send_criu_rpc_script(enum script_actions act, char *name, int sk, int fd, char *key)
> {
> int ret;
> + int ret_fd = 0;
> CriuResp msg = CRIU_RESP__INIT;
> CriuReq *req;
> CriuNotify cn = CRIU_NOTIFY__INIT;
> @@ -211,6 +212,15 @@ int send_criu_rpc_script(enum script_actions act, char *name, int sk, int fd)
> cn.has_pid = true;
> cn.pid = root_item->pid->real;
> break;
> + case ACT_REQ_INHERIT_FD:
> + /*
> + * Sending a 'ACT_REQ_INHERIT_FD' notify message
> + * only makes sense when 'key' is set.
> + */
> + if (!key)
> + return -1;
> + cn.inherit_fd_key = key;
> + break;
> default:
> break;
> }
> @@ -219,6 +229,18 @@ int send_criu_rpc_script(enum script_actions act, char *name, int sk, int fd)
> if (ret < 0)
> return ret;
>
> + if (act == ACT_REQ_INHERIT_FD) {
> + /*
> + * Sending a ACT_REQ_INHERIT_FD notify message means, that CRIU
> + * expects to get a FD from the RPC client.
> + */
> + ret_fd = recv_fd(sk);
> + if (ret_fd <= 0) {
> + pr_perror("recv_fd error\n");
> + return -1;
> + }
> + }
> +
> ret = recv_criu_msg(sk, &req);
> if (ret < 0)
> return ret;
> @@ -229,7 +251,7 @@ int send_criu_rpc_script(enum script_actions act, char *name, int sk, int fd)
> }
>
> criu_req__free_unpacked(req, NULL);
> - return 0;
> + return ret_fd;
> }
>
> static char images_dir[PATH_MAX];
> @@ -481,11 +503,21 @@ static int setup_opts_from_req(int sk, CriuOpts *req)
> }
>
> if (req->n_inherit_fd && !opts.swrk_restore) {
> + /* TODO: Is this really still true, even with callback handling for inherit_fd */
> pr_err("inherit_fd is not allowed in standalone service\n");
> goto err;
> }
> for (i = 0; i < req->n_inherit_fd; i++) {
> - if (inherit_fd_add(req->inherit_fd[i]->fd, req->inherit_fd[i]->key))
> + /*
> + * For the FDs 0, 1, 2 this falls back to old, non callbacb
> + * inherit_fd mode. At some point in the future this can be removed.
> + */
> + if (req->inherit_fd[i]->fd >=0 && req->inherit_fd[i]->fd <= 2) {
We can add the following patch and handle all non-negative fd
diff --git a/images/rpc.proto b/images/rpc.proto
index 33ef3302b..04b933674 100644
--- a/images/rpc.proto
+++ b/images/rpc.proto
@@ -25,7 +25,7 @@ message join_namespace {
message inherit_fd {
required string key = 1;
- required int32 fd = 2;
+ required int32 fd = 2 [default = -1];
};
message cgroup_root {
> + if (inherit_fd_add(req->inherit_fd[i]->fd, req->inherit_fd[i]->key))
> + goto err;
> + continue;
> + }
> + if (inherit_fd_add_rpc(req->inherit_fd[i]->key))
I think you misunderstood me. I was thinking to request an inherit fd from
inherit_fd_lookup_id() to avoid fd clashes on restore
(inherit_fd_resolve_clash). It will work this way too, but in case of
swrk, it will be more efficiant just to run criu swrk will all required
fds (this way is handled in a previous "if").
Thanks,
Andrei
> goto err;
> }
>
> diff --git a/criu/files.c b/criu/files.c
> index 4ed84305e..6d71bb9df 100644
> --- a/criu/files.c
> +++ b/criu/files.c
> @@ -26,6 +26,7 @@
> #include "sockets.h"
> #include "pstree.h"
> #include "tty.h"
> +#include "action-scripts.h"
> #include "pipes.h"
> #include "fifo.h"
> #include "eventfd.h"
> @@ -1572,6 +1573,15 @@ int inherit_fd_parse(char *optarg)
> return inherit_fd_add(fd, cp);
> }
>
> +int inherit_fd_add_rpc(char *key)
> +{
> + int fd;
> + fd = rpc_send_fd(ACT_REQ_INHERIT_FD, -1, key);
> + if (fd <= 0)
> + return -1;
> + return inherit_fd_add(fd, key);
> +}
> +
> int inherit_fd_add(int fd, char *key)
> {
> struct inherit_fd *inh;
> diff --git a/criu/include/action-scripts.h b/criu/include/action-scripts.h
> index 4b90feb92..bd96e256d 100644
> --- a/criu/include/action-scripts.h
> +++ b/criu/include/action-scripts.h
> @@ -23,7 +23,7 @@ enum script_actions {
> extern int add_script(char *path);
> extern int add_rpc_notify(int sk);
> extern int run_scripts(enum script_actions);
> -extern int rpc_send_fd(enum script_actions, int fd);
> -extern int send_criu_rpc_script(enum script_actions act, char *name, int sk, int fd);
> +extern int rpc_send_fd(enum script_actions, int fd, char *key);
> +extern int send_criu_rpc_script(enum script_actions act, char *name, int sk, int fd, char *key);
>
> #endif /* __CR_ACTION_SCRIPTS_H__ */
> diff --git a/criu/include/files.h b/criu/include/files.h
> index 75757bd1d..9cffe960c 100644
> --- a/criu/include/files.h
> +++ b/criu/include/files.h
> @@ -188,6 +188,7 @@ extern int dump_unsupp_fd(struct fd_parms *p, int lfd,
> char *more, char *info, FdinfoEntry *);
>
> extern int inherit_fd_parse(char *optarg);
> +extern int inherit_fd_add_rpc(char *key);
> extern int inherit_fd_add(int fd, char *key);
> extern void inherit_fd_log(void);
> extern int inherit_fd_resolve_clash(int fd);
> diff --git a/criu/tty.c b/criu/tty.c
> index 30e3d7288..676ad888c 100644
> --- a/criu/tty.c
> +++ b/criu/tty.c
> @@ -984,7 +984,7 @@ static int pty_open_unpaired_slave(struct file_desc *d, struct tty_info *slave)
> unlock_pty(master);
>
> if (opts.orphan_pts_master &&
> - rpc_send_fd(ACT_ORPHAN_PTS_MASTER, master) == 0) {
> + rpc_send_fd(ACT_ORPHAN_PTS_MASTER, master, NULL) == 0) {
>
> fd = open_tty_reg(slave->reg_d, slave->tfe->flags);
> if (fd < 0) {
> --
> 2.18.0
>
More information about the CRIU
mailing list