From patchwork Mon Aug 6 21:01:05 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [4/4] cr-service: use inherit-fd keys to ask for FD From: Andrei Vagin X-Patchwork-Id: 9049 Message-Id: <20180806210104.GA10573@outlook.office365.com> To: Adrian Reber Cc: criu@openvz.org, Adrian Reber Date: Mon, 6 Aug 2018 14:01:05 -0700 On Mon, Aug 06, 2018 at 04:55:00PM +0000, Adrian Reber wrote: > From: Adrian Reber > > 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 > --- > 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 > + 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 > 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 {