[CRIU] [PATCH 3/4] remap: add a dead pid /proc remap
Tycho Andersen
tycho.andersen at canonical.com
Thu Sep 18 06:41:00 PDT 2014
On Thu, Sep 18, 2014 at 03:03:15PM +0400, Pavel Emelyanov wrote:
> On 09/18/2014 12:32 AM, Tycho Andersen wrote:
> > If a file like /proc/20/mountinfo is open, but 20 is a zombie (or doesn't exist
> > any more), we can't read this file at all, so a link remap won't work. Instead,
> > we add a new remap, called the dead process remap, which forks a TASK_HELPER as
> > that dead pid so that the restore task can open the new /proc/20/mountinfo
> > instead.
> >
> > This commit also adds a new stage CR_STATE_RESTORE_SHARED. Since new
> > TASK_HELPERS are added when loading the shared resource images, we need to wait
> > to start forking tasks until after these resources are loaded.
>
> I don't quite understand this explanation. CRIU forks root task, which calls
> root_prepare_shared() in which all the remaps are collected. At that point any
> proc remap is lined into the pstree. After this root task starts forking
> whatever entries it finds in there. Why do we need one more stage?
Because of restore_root_task. Think of the case when after the
CR_STATE_RESTORE_NS, the restore_root_task process runs until it
starts waiting for CR_STATE_FORKING. It enters that wait looking for
task_entries->nr_tasks + task_entries->nr_helpers. But then the init
task runs, parses the shared state and adds some more helpers. The
first thread was waiting on the wrong number of helpers.
Another option would be to get rid of the CR_STATE_RESTORE_NS point
and just synchronize after restoring the shared state. Then the
ACT_SETUP_NS scripts wouldn't necessarily run before the tasks are
forked, though. I didn't know if that was a problem or not, so I left
it the way it was.
Tycho
> > v2: fix a mutex bug
> >
> > Signed-off-by: Tycho Andersen <tycho.andersen at canonical.com>
> > ---
> > cr-restore.c | 7 ++++
> > files-reg.c | 80 ++++++++++++++++++++++++++++++++++++++++++
> > include/fs-magic.h | 4 +++
> > include/restorer.h | 1 +
> > protobuf/remap-file-path.proto | 1 +
> > 5 files changed, 93 insertions(+)
> >
> > diff --git a/cr-restore.c b/cr-restore.c
> > index 7da6c3f..0a42684 100644
> > --- a/cr-restore.c
> > +++ b/cr-restore.c
> > @@ -1420,6 +1420,9 @@ static int restore_task_with_children(void *_arg)
> >
> > if (root_prepare_shared())
> > goto err_fini_mnt;
> > +
> > + if (restore_finish_stage(CR_STATE_RESTORE_SHARED) < 0)
> > + goto err_fini_mnt;
> > }
> >
> > if (prepare_mappings(pid))
> > @@ -1489,6 +1492,7 @@ static inline int stage_participants(int next_stage)
> > case CR_STATE_FAIL:
> > return 0;
> > case CR_STATE_RESTORE_NS:
> > + case CR_STATE_RESTORE_SHARED:
> > return 1;
> > case CR_STATE_FORKING:
> > return task_entries->nr_tasks + task_entries->nr_helpers;
> > @@ -1700,6 +1704,9 @@ static int restore_root_task(struct pstree_item *init)
> > goto out;
> >
> > timing_start(TIME_FORK);
> > + ret = restore_switch_stage(CR_STATE_RESTORE_SHARED);
> > + if (ret < 0)
> > + goto out;
> >
> > ret = restore_switch_stage(CR_STATE_FORKING);
> > if (ret < 0)
> > diff --git a/files-reg.c b/files-reg.c
> > index bb16e5f..508263a 100644
> > --- a/files-reg.c
> > +++ b/files-reg.c
> > @@ -24,6 +24,7 @@
> > #include "asm/atomic.h"
> > #include "namespaces.h"
> > #include "proc_parse.h"
> > +#include "pstree.h"
> >
> > #include "protobuf.h"
> > #include "protobuf/regfile.pb-c.h"
> > @@ -226,6 +227,36 @@ static int open_remap_linked(struct reg_file_info *rfi,
> > return 0;
> > }
> >
> > +static int open_remap_dead_process(struct reg_file_info *rfi,
> > + RemapFilePathEntry *rfe)
> > +{
> > + struct pstree_item *helper;
> > +
> > + for_each_pstree_item(helper) {
> > + /* don't need to add multiple tasks */
> > + if (helper->pid.virt == rfe->remap_id) {
> > + pr_info("Skipping helper for restoring /proc/%d; pid exists\n", rfe->remap_id);
> > + return 0;
> > + }
> > + }
> > +
> > + helper = alloc_pstree_item_with_rst();
> > + if (!helper)
> > + return -1;
> > +
> > + helper->sid = root_item->sid;
> > + helper->pgid = root_item->pgid;
> > + helper->pid.virt = rfe->remap_id;
> > + helper->state = TASK_HELPER;
> > + helper->parent = root_item;
> > + list_add_tail(&helper->sibling, &root_item->children);
> > + task_entries->nr_helpers++;
> > +
> > + pr_info("Added a helper for restoring /proc/%d\n", helper->pid.virt);
> > +
> > + return 0;
> > +}
> > +
> > static int collect_one_remap(void *obj, ProtobufCMessage *msg)
> > {
> > int ret = -1;
> > @@ -263,6 +294,9 @@ static int collect_one_remap(void *obj, ProtobufCMessage *msg)
> > case REMAP_TYPE__GHOST:
> > ret = open_remap_ghost(rfi, rfe);
> > break;
> > + case REMAP_TYPE__PROCFS:
> > + ret = open_remap_dead_process(rfi, rfe);
> > + break;
> > default:
> > pr_err("unknown remap type %u\n", rfe->remap_type);
> > goto out;
> > @@ -516,6 +550,20 @@ static int dump_linked_remap(char *path, int len, const struct stat *ost,
> > &rpe, PB_REMAP_FPATH);
> > }
> >
> > +static int dump_dead_process_remap(pid_t pid, char *path, int len, const struct stat *ost,
> > + int lfd, u32 id, struct ns_id *nsid)
> > +{
> > + RemapFilePathEntry rpe = REMAP_FILE_PATH_ENTRY__INIT;
> > +
> > + rpe.orig_id = id;
> > + rpe.remap_id = pid;
> > + rpe.has_remap_type = true;
> > + rpe.remap_type = REMAP_TYPE__PROCFS;
> > +
> > + return pb_write_one(fdset_fd(glob_fdset, CR_FD_REMAP_FPATH),
> > + &rpe, PB_REMAP_FPATH);
> > +}
> > +
> > static bool is_sillyrename_name(char *name)
> > {
> > int i;
> > @@ -557,6 +605,38 @@ static int check_path_remap(char *rpath, int plen, const struct fd_parms *parms,
> > struct stat pst;
> > const struct stat *ost = &parms->stat;
> >
> > + if (parms->fs_type == PROC_SUPER_MAGIC) {
> > + /* The file points to /proc/pid/<foo> where pid is a dead
> > + * process. We remap this file by adding this pid to be
> > + * fork()ed into a TASK_HELPER state so that we can point to it
> > + * on restore.
> > + */
> > + pid_t pid;
> > + char *start, *end;
> > +
> > + /* skip "./proc/" */
> > + start = strstr(rpath, "/") + 1;
> > + if (!start)
> > + return -1;
> > + start = strstr(start, "/") + 1;
> > + if (!start)
> > + return -1;
> > + pid = strtol(start, &end, 10);
> > +
> > + /* if we didn't find another /, this path something
> > + * like ./proc/kmsg, which we shouldn't mess with. */
> > + if (*end == '/') {
> > + *end = 0;
> > + ret = access(rpath, F_OK);
> > + *end = '/';
> > +
> > + if (ret) {
> > + pr_info("Dumping dead process remap of %d\n", pid);
> > + return dump_dead_process_remap(pid, rpath + 1, plen - 1, ost, lfd, id, nsid);
> > + }
> > + }
> > + }
> > +
> > if (ost->st_nlink == 0)
> > /*
> > * Unpleasant, but easy case. File is completely invisible
> > diff --git a/include/fs-magic.h b/include/fs-magic.h
> > index 13c4961..777d736 100644
> > --- a/include/fs-magic.h
> > +++ b/include/fs-magic.h
> > @@ -41,4 +41,8 @@
> > #define AUFS_SUPER_MAGIC 0x61756673
> > #endif
> >
> > +#ifndef PROC_SUPER_MAGIC
> > +#define PROC_SUPER_MAGIC 0x9fa0
> > +#endif
> > +
> > #endif /* __CR_FS_MAGIC_H__ */
> > diff --git a/include/restorer.h b/include/restorer.h
> > index 0d9f961..107c7d2 100644
> > --- a/include/restorer.h
> > +++ b/include/restorer.h
> > @@ -171,6 +171,7 @@ static inline unsigned long restorer_stack(struct thread_restore_args *a)
> > enum {
> > CR_STATE_FAIL = -1,
> > CR_STATE_RESTORE_NS = 0, /* is used for executing "setup-namespace" scripts */
> > + CR_STATE_RESTORE_SHARED,
> > CR_STATE_FORKING,
> > CR_STATE_RESTORE,
> > CR_STATE_RESTORE_SIGCHLD,
> > diff --git a/protobuf/remap-file-path.proto b/protobuf/remap-file-path.proto
> > index 37d50e5..6854974 100644
> > --- a/protobuf/remap-file-path.proto
> > +++ b/protobuf/remap-file-path.proto
> > @@ -1,6 +1,7 @@
> > enum remap_type {
> > LINKED = 0;
> > GHOST = 1;
> > + PROCFS = 2;
> > };
> >
> > message remap_file_path_entry {
> >
>
More information about the CRIU
mailing list