[CRIU] Re: [PATCH cr 02/10] pstree: simplify access to pid, real_pid, born_sid

Pavel Emelyanov xemul at parallels.com
Tue Jun 19 14:26:50 EDT 2012


On 06/19/2012 04:46 PM, Andrey Vagin wrote:
> I'm bored to write pi->pid.pid or pi->pid.read_pid. 

Shit :( How about

struct pid {
	u32	r; /* real */
	u32	v; /* virtual */
};

s/->pid.pid/->pid.r/
s/->pid.real_pid/->pid.v/

?

> struct pid is required in one place for restoring name-spaces and it will be created
> there.
> 
> born_sid will be appear in folloing patches.
> 
> Signed-off-by: Andrey Vagin <avagin at openvz.org>
> ---
>  cr-dump.c         |   70 +++++++++++++++++++++++++++-------------------------
>  cr-restore.c      |   38 ++++++++++++++--------------
>  cr-show.c         |    8 +++---
>  files.c           |    4 +-
>  include/crtools.h |    8 +++---
>  5 files changed, 65 insertions(+), 63 deletions(-)
> 
> diff --git a/cr-dump.c b/cr-dump.c
> index 4db19cf..6880740 100644
> --- a/cr-dump.c
> +++ b/cr-dump.c
> @@ -980,7 +980,7 @@ static int parse_threads(const struct pstree_item *item, struct pid **_t, int *_
>         struct pid *t = NULL;
>         int nr = 1;
> 
> -       dir = opendir_proc(item->pid.real_pid, "task");
> +       dir = opendir_proc(item->real_pid, "task");
>         if (!dir)
>                 return -1;
> 
> @@ -1043,7 +1043,7 @@ static int parse_children(const struct pstree_item *item, u32 **_c, int *_n)
>         int nr = 1, i;
> 
>         for (i = 0; i < item->nr_threads; i++) {
> -               file = fopen_proc(item->pid.real_pid, "task/%d/children",
> +               file = fopen_proc(item->real_pid, "task/%d/children",
>                                                 item->threads[i].real_pid);
>                 if (!file)
>                         goto err;
> @@ -1087,8 +1087,8 @@ struct pstree_item *__alloc_pstree_item(bool rst)
>         INIT_LIST_HEAD(&item->children);
>         item->threads = NULL;
>         item->nr_threads = 0;
> -       item->pid.pid = -1;
> -       item->pid.real_pid = -1;
> +       item->pid = -1;
> +       item->real_pid = -1;
> 
>         return item;
>  }
> @@ -1109,7 +1109,7 @@ static int get_children(struct pstree_item *item)
>                         ret = -1;
>                         goto free;
>                 }
> -               c->pid.real_pid = ch[i];
> +               c->real_pid = ch[i];
>                 c->parent = item;
>                 list_add_tail(&c->list, &item->children);
>         }
> @@ -1162,7 +1162,7 @@ static void pstree_switch_state(struct pstree_item *root_item, int st)
>  static pid_t item_ppid(const struct pstree_item *item)
>  {
>         item = item->parent;
> -       return item ? item->pid.real_pid : -1;
> +       return item ? item->real_pid : -1;
>  }
> 
>  static int seize_threads(const struct pstree_item *item)
> @@ -1176,11 +1176,11 @@ static int seize_threads(const struct pstree_item *item)
> 
>         for (i = 0; i < item->nr_threads; i++) {
>                 pid_t pid = item->threads[i].real_pid;
> -               if (item->pid.real_pid == pid)
> +               if (item->real_pid == pid)
>                         continue;
> 
>                 pr_info("\tSeizing %d's %d thread\n",
> -                               item->pid.real_pid, pid);
> +                               item->real_pid, pid);
>                 ret = seize_task(pid, item_ppid(item), NULL, NULL);
>                 if (ret < 0)
>                         goto err;
> @@ -1200,7 +1200,7 @@ static int seize_threads(const struct pstree_item *item)
> 
>  err:
>         for (i--; i >= 0; i--) {
> -               if (item->pid.real_pid == item->threads[i].real_pid)
> +               if (item->real_pid == item->threads[i].real_pid)
>                         continue;
> 
>                 unseize_task(item->threads[i].real_pid, TASK_ALIVE);
> @@ -1255,20 +1255,20 @@ static int check_xids(struct pstree_item *root_item)
>                         continue;
> 
>                 /* Easing #1 and #2 for sids */
> -               if ((p->sid != p->pid.pid) && (p->sid != p->parent->sid)) {
> +               if ((p->sid != p->pid) && (p->sid != p->parent->sid)) {
>                         pr_err("SID mismatch on %d (%d/%d)\n",
> -                                       p->pid.pid, p->sid, p->parent->sid);
> +                                       p->pid, p->sid, p->parent->sid);
>                         return -1;
>                 }
> 
>                 /* Easing #2 for pgids */
>                 for_each_pstree_item(tmp)
> -                       if (tmp->pid.pid == p->pgid)
> +                       if (tmp->pid == p->pgid)
>                                 break;
> 
>                 if (tmp == NULL) {
>                         pr_err("PGIG mismatch on %d (%d)\n",
> -                                       p->pid.pid, p->pgid);
> +                                       p->pid, p->pgid);
>                         return -1;
>                 }
>         }
> @@ -1279,7 +1279,7 @@ static int check_xids(struct pstree_item *root_item)
>  static int collect_task(struct pstree_item *item)
>  {
>         int ret;
> -       pid_t pid = item->pid.real_pid;
> +       pid_t pid = item->real_pid;
> 
>         ret = seize_task(pid, item_ppid(item), &item->pgid, &item->sid);
>         if (ret < 0)
> @@ -1303,7 +1303,7 @@ static int collect_task(struct pstree_item *item)
> 
>         close_pid_proc();
> 
> -       pr_info("Collected %d in %d state\n", item->pid.real_pid, item->state);
> +       pr_info("Collected %d in %d state\n", item->real_pid, item->state);
>         return 0;
> 
>  err_close:
> @@ -1325,7 +1325,7 @@ static int check_subtree(const struct pstree_item *item)
> 
>         i = 0;
>         list_for_each_entry(child, &item->children, list) {
> -               if (child->pid.real_pid != ch[i])
> +               if (child->real_pid != ch[i])
>                         break;
>                 i++;
>                 if (i > nr)
> @@ -1344,7 +1344,7 @@ static int check_subtree(const struct pstree_item *item)
>  static int collect_subtree(struct pstree_item *item, int leader_only)
>  {
>         struct pstree_item *child;
> -       pid_t pid = item->pid.real_pid;
> +       pid_t pid = item->real_pid;
>         int ret;
> 
>         pr_info("Collecting tasks starting from %d\n", pid);
> @@ -1376,7 +1376,7 @@ static int collect_pstree(pid_t pid, const struct cr_options *opts)
>                 if (root_item == NULL)
>                         return -1;
> 
> -               root_item->pid.real_pid = pid;
> +               root_item->real_pid = pid;
>                 INIT_LIST_HEAD(&root_item->list);
> 
>                 ret = collect_subtree(root_item, opts->leader_only);
> @@ -1386,7 +1386,7 @@ static int collect_pstree(pid_t pid, const struct cr_options *opts)
>                          * namespaces' reaper. Check this.
>                          */
>                         if (opts->namespaces_flags & CLONE_NEWPID) {
> -                               BUG_ON(root_item->pid.real_pid != 1);
> +                               BUG_ON(root_item->real_pid != 1);
> 
>                                 if (check_subtree(root_item))
>                                         goto try_again;
> @@ -1423,7 +1423,7 @@ static int dump_pstree(struct pstree_item *root_item)
>         int pstree_fd;
> 
>         pr_info("\n");
> -       pr_info("Dumping pstree (pid: %d)\n", root_item->pid.real_pid);
> +       pr_info("Dumping pstree (pid: %d)\n", root_item->real_pid);
>         pr_info("----------------------------------------\n");
> 
>         ret = check_xids(root_item);
> @@ -1435,10 +1435,10 @@ static int dump_pstree(struct pstree_item *root_item)
>                 return -1;
> 
>         for_each_pstree_item(item) {
> -               pr_info("Process: %d(%d)\n", item->pid.pid, item->pid.real_pid);
> +               pr_info("Process: %d(%d)\n", item->pid, item->real_pid);
> 
> -               e.pid           = item->pid.pid;
> -               e.ppid          = item->parent ? item->parent->pid.pid : 0;
> +               e.pid           = item->pid;
> +               e.ppid          = item->parent ? item->parent->pid : 0;
>                 e.pgid          = item->pgid;
>                 e.sid           = item->sid;
>                 e.nr_threads    = item->nr_threads;
> @@ -1520,7 +1520,7 @@ static int dump_one_zombie(const struct pstree_item *item,
>         core->tc.task_state = TASK_DEAD;
>         core->tc.exit_code = pps->exit_code;
> 
> -       fd_core = open_image(CR_FD_CORE, O_DUMP, item->pid.pid);
> +       fd_core = open_image(CR_FD_CORE, O_DUMP, item->pid);
>         if (fd_core < 0)
>                 goto err_free;
> 
> @@ -1541,8 +1541,8 @@ static int dump_task_threads(struct parasite_ctl *parasite_ctl,
> 
>         for (i = 0; i < item->nr_threads; i++) {
>                 /* Leader is already dumped */
> -               if (item->pid.real_pid == item->threads[i].real_pid) {
> -                       item->threads[i].pid = item->pid.pid;
> +               if (item->real_pid == item->threads[i].real_pid) {
> +                       item->threads[i].pid = item->pid;
>                         continue;
>                 }
> 
> @@ -1555,7 +1555,7 @@ static int dump_task_threads(struct parasite_ctl *parasite_ctl,
> 
>  static int dump_one_task(struct pstree_item *item)
>  {
> -       pid_t pid = item->pid.real_pid;
> +       pid_t pid = item->real_pid;
>         LIST_HEAD(vma_area_list);
>         struct parasite_ctl *parasite_ctl;
>         int ret = -1;
> @@ -1585,12 +1585,12 @@ static int dump_one_task(struct pstree_item *item)
> 
>         if (item->state == TASK_DEAD) {
>                 /* FIXME don't support zombie in pid name space*/
> -               if (root_item->pid.pid == 1) {
> -                       pr_err("Can't dump a zombie %d in PIDNS", item->pid.real_pid);
> +               if (root_item->pid == 1) {
> +                       pr_err("Can't dump a zombie %d in PIDNS", item->real_pid);
>                         ret = -1;
>                         goto err;
>                 }
> -               item->pid.pid = item->pid.real_pid;
> +               item->pid = item->real_pid;
> 
>                 BUG_ON(!list_empty(&item->children));
>                 return dump_one_zombie(item, &pps_buf);
> @@ -1621,12 +1621,12 @@ static int dump_one_task(struct pstree_item *item)
>                 goto err_cure_fdset;
>         }
> 
> -       item->pid.pid = misc.pid;
> +       item->pid = misc.pid;
>         item->sid = misc.sid;
>         item->pgid = misc.pgid;
> 
>         ret = -1;
> -       cr_fdset = cr_task_fdset_open(item->pid.pid, O_DUMP);
> +       cr_fdset = cr_task_fdset_open(item->pid, O_DUMP);
>         if (!cr_fdset)
>                 goto err_cure;
> 
> @@ -1747,9 +1747,11 @@ int cr_dump_tasks(pid_t pid, const struct cr_options *opts)
>         if (dump_pstree(root_item))
>                 goto err;
> 
> -       if (opts->namespaces_flags)
> -               if (dump_namespaces(&root_item->pid, opts->namespaces_flags) < 0)
> +       if (opts->namespaces_flags) {
> +               struct pid pid_ns = {root_item->pid, root_item->real_pid};
> +               if (dump_namespaces(&pid_ns, opts->namespaces_flags) < 0)
>                         goto err;
> +       }
> 
>         ret = cr_dump_shmem();
>         if (ret)
> diff --git a/cr-restore.c b/cr-restore.c
> index d5c15d9..ce28b44 100644
> --- a/cr-restore.c
> +++ b/cr-restore.c
> @@ -100,7 +100,7 @@ static int prepare_pstree(void)
>                 if (pi == NULL)
>                         break;
> 
> -               pi->pid.pid = e.pid;
> +               pi->pid = e.pid;
>                 pi->pgid = e.pgid;
>                 pi->sid = e.sid;
> 
> @@ -116,18 +116,18 @@ static int prepare_pstree(void)
>                          * and sit among the last item's ancestors.
>                          */
>                         while (parent) {
> -                               if (parent->pid.pid == e.ppid)
> +                               if (parent->pid == e.ppid)
>                                         break;
>                                 parent = parent->parent;
>                         }
> 
>                         if (parent == NULL)
>                                 for_each_pstree_item(parent)
> -                                       if (parent->pid.pid == e.ppid)
> +                                       if (parent->pid == e.ppid)
>                                                 break;
> 
>                         if (parent == NULL) {
> -                               pr_err("Can't find a parent for %d", pi->pid.pid);
> +                               pr_err("Can't find a parent for %d", pi->pid);
>                                 xfree(pi);
>                                 break;
>                         }
> @@ -201,11 +201,11 @@ static int prepare_shared(void)
>                 return -1;
> 
>         for_each_pstree_item(pi) {
> -               ret = prepare_shmem_pid(pi->pid.pid);
> +               ret = prepare_shmem_pid(pi->pid);
>                 if (ret < 0)
>                         break;
> 
> -               ret = prepare_fd_pid(pi->pid.pid, pi->rst);
> +               ret = prepare_fd_pid(pi->pid, pi->rst);
>                 if (ret < 0)
>                         break;
>         }
> @@ -497,7 +497,7 @@ static inline int fork_with_pid(struct pstree_item *item, unsigned long ns_clone
>         char buf[32];
>         struct cr_clone_arg ca;
>         void *stack;
> -       pid_t pid = item->pid.pid;
> +       pid_t pid = item->pid;
> 
>         pr_info("Forking task with %d pid (flags 0x%lx)\n", pid, ns_clone_flags);
> 
> @@ -581,8 +581,8 @@ static void restore_sid(void)
>          * we can call setpgid() on custom values.
>          */
> 
> -       pr_info("Restoring %d to %d sid\n", me->pid.pid, me->sid);
> -       if (me->pid.pid == me->sid) {
> +       pr_info("Restoring %d to %d sid\n", me->pid, me->sid);
> +       if (me->pid == me->sid) {
>                 sid = setsid();
>                 if (sid != me->sid) {
>                         pr_perror("Can't restore sid (%d)", sid);
> @@ -602,7 +602,7 @@ static void restore_pgid(void)
>  {
>         pid_t pgid;
> 
> -       pr_info("Restoring %d to %d pgid\n", me->pid.pid, me->pgid);
> +       pr_info("Restoring %d to %d pgid\n", me->pid, me->pgid);
> 
>         pgid = getpgrp();
>         if (me->pgid == pgid)
> @@ -610,7 +610,7 @@ static void restore_pgid(void)
> 
>         pr_info("\twill call setpgid, mine pgid is %d\n", pgid);
>         if (setpgid(0, me->pgid) != 0) {
> -               pr_perror("Can't restore pgid (%d/%d->%d)", me->pid.pid, pgid, me->pgid);
> +               pr_perror("Can't restore pgid (%d/%d->%d)", me->pid, pgid, me->pgid);
>                 xid_fail();
>         }
>  }
> @@ -630,8 +630,8 @@ static int restore_task_with_children(void *_arg)
>         me = ca->item;
> 
>         pid = getpid();
> -       if (me->pid.pid != pid) {
> -               pr_err("Pid %d do not match expected %d\n", pid, me->pid.pid);
> +       if (me->pid != pid) {
> +               pr_err("Pid %d do not match expected %d\n", pid, me->pid);
>                 exit(-1);
>         }
> 
> @@ -649,7 +649,7 @@ static int restore_task_with_children(void *_arg)
>                 exit(1);
> 
>         if (ca->clone_flags) {
> -               ret = prepare_namespace(me->pid.pid, ca->clone_flags);
> +               ret = prepare_namespace(me->pid, ca->clone_flags);
>                 if (ret)
>                         exit(-1);
>         }
> @@ -665,7 +665,7 @@ static int restore_task_with_children(void *_arg)
>         sigdelset(&blockmask, SIGCHLD);
>         ret = sigprocmask(SIG_BLOCK, &blockmask, NULL);
>         if (ret) {
> -               pr_perror("%d: Can't block signals", me->pid.pid);
> +               pr_perror("%d: Can't block signals", me->pid);
>                 exit(1);
>         }
> 
> @@ -681,7 +681,7 @@ static int restore_task_with_children(void *_arg)
> 
>         restore_pgid();
> 
> -       return restore_one_task(me->pid.pid);
> +       return restore_one_task(me->pid);
>  }
> 
>  static int restore_root_task(struct pstree_item *init, struct cr_options *opts)
> @@ -710,7 +710,7 @@ static int restore_root_task(struct pstree_item *init, struct cr_options *opts)
>          * this later.
>          */
> 
> -       if (init->pid.pid == 1) {
> +       if (init->pid == 1) {
>                 sprintf(proc_mountpoint, "/tmp/crtools-proc.XXXXXX");
>                 if (mkdtemp(proc_mountpoint) == NULL) {
>                         pr_err("mkdtemp failed %m");
> @@ -740,7 +740,7 @@ static int restore_root_task(struct pstree_item *init, struct cr_options *opts)
>         ret = (int)futex_get(&task_entries->nr_in_progress);
> 
>  out:
> -       if (init->pid.pid == 1) {
> +       if (init->pid == 1) {
>                 int err;
>                 err = umount(proc_mountpoint);
>                 if (err == -1)
> @@ -755,7 +755,7 @@ out:
>                 pr_err("Someone can't be restored\n");
> 
>                 for_each_pstree_item(pi)
> -                       kill(pi->pid.pid, SIGKILL);
> +                       kill(pi->pid, SIGKILL);
> 
>                 return 1;
>         }
> diff --git a/cr-show.c b/cr-show.c
> index f697bcc..973429c 100644
> --- a/cr-show.c
> +++ b/cr-show.c
> @@ -426,7 +426,7 @@ static int show_collect_pstree(int fd_pstree, struct list_head *collect)
>                         if (!item)
>                                 return -1;
> 
> -                       item->pid.pid = e.pid;
> +                       item->pid = e.pid;
>                         item->nr_threads = e.nr_threads;
>                         item->threads = xzalloc(sizeof(u32) * e.nr_threads);
>                         if (!item->threads) {
> @@ -649,7 +649,7 @@ static int cr_show_all(struct cr_options *opts)
>         show_sk_queues(fd, opts);
>         close(fd);
> 
> -       pid = list_first_entry(&pstree_list, struct pstree_item, list)->pid.pid;
> +       pid = list_first_entry(&pstree_list, struct pstree_item, list)->pid;
>         ret = try_show_namespaces(pid, opts);
>         if (ret)
>                 goto out;
> @@ -657,7 +657,7 @@ static int cr_show_all(struct cr_options *opts)
>         list_for_each_entry(item, &pstree_list, list) {
>                 struct cr_fdset *cr_fdset = NULL;
> 
> -               cr_fdset = cr_task_fdset_open(item->pid.pid, O_SHOW);
> +               cr_fdset = cr_task_fdset_open(item->pid, O_SHOW);
>                 if (!cr_fdset)
>                         goto out;
> 
> @@ -668,7 +668,7 @@ static int cr_show_all(struct cr_options *opts)
> 
>                         for (i = 0; i < item->nr_threads; i++) {
> 
> -                               if (item->threads[i].pid == item->pid.pid)
> +                               if (item->threads[i].pid == item->pid)
>                                         continue;
> 
>                                 fd_th = open_image_ro(CR_FD_CORE, item->threads[i]);
> diff --git a/files.c b/files.c
> index f30aee1..599281b 100644
> --- a/files.c
> +++ b/files.c
> @@ -663,7 +663,7 @@ int prepare_fds(struct pstree_item *me)
> 
>         for (state = 0; state < FD_STATE_MAX; state++) {
>                 list_for_each_entry(fle, &me->rst->fds, ps_list) {
> -                       ret = open_fdinfo(me->pid.pid, &fle->fe, state);
> +                       ret = open_fdinfo(me->pid, &fle->fe, state);
>                         if (ret)
>                                 goto done;
>                 }
> @@ -674,7 +674,7 @@ int prepare_fds(struct pstree_item *me)
>                  * list and restore at the very end.
>                  */
>                 list_for_each_entry(fle, &me->rst->eventpoll, ps_list) {
> -                       ret = open_fdinfo(me->pid.pid, &fle->fe, state);
> +                       ret = open_fdinfo(me->pid, &fle->fe, state);
>                         if (ret)
>                                 goto done;
>                 }
> diff --git a/include/crtools.h b/include/crtools.h
> index d99e5ab..cb7dd3a 100644
> --- a/include/crtools.h
> +++ b/include/crtools.h
> @@ -176,15 +176,15 @@ struct rst_info {
>         struct list_head        eventpoll;
>  };
> 
> -struct pid
> -{
> -       u32 real_pid;
> +struct pid {
>         u32 pid;
> +       u32 real_pid;
>  };
> 
>  struct pstree_item {
>         struct list_head        list;
> -       struct pid              pid;            /* leader pid */
> +       u32 pid;
> +       u32 real_pid;
>         struct pstree_item      *parent;
>         struct list_head        children;       /* array of children */
>         pid_t                   pgid;
> --
> 1.7.1
> 
> .
> 



More information about the CRIU mailing list