| Message ID | 20190118004314.32680-1-rstoyanov1@gmail.com |
|---|---|
| State | Accepted |
| Series | "pstree: Restore task group leader when inherit SID" |
| Headers | show |
diff --git a/criu/pstree.c b/criu/pstree.c index 695110c45..92b4167aa 100644 --- a/criu/pstree.c +++ b/criu/pstree.c @@ -367,22 +367,31 @@ static int prepare_pstree_for_shell_job(pid_t pid) */ old_sid = root_item->sid; - old_gid = root_item->pgid; - pr_info("Migrating process tree (GID %d->%d SID %d->%d)\n", - old_gid, current_gid, old_sid, current_sid); + pr_info("Migrating process tree (SID %d->%d)\n", + old_sid, current_sid); for_each_pstree_item(pi) { - if (pi->pgid == old_gid) - pi->pgid = current_gid; if (pi->sid == old_sid) pi->sid = current_sid; } - if (lookup_create_item(current_sid) == NULL) - return -1; - if (lookup_create_item(current_gid) == NULL) - return -1; + old_gid = root_item->pgid; + if (old_gid != vpid(root_item)) { + if (lookup_create_item(current_sid) == NULL) + return -1; + + pr_info("Migrating process tree (GID %d->%d)\n", + old_gid, current_gid); + + for_each_pstree_item(pi) { + if (pi->pgid == old_gid) + pi->pgid = current_gid; + } + + if (lookup_create_item(current_gid) == NULL) + return -1; + } return 0; } diff --git a/criu/tty.c b/criu/tty.c index 38e1cab33..5aaca633a 100644 --- a/criu/tty.c +++ b/criu/tty.c @@ -1068,10 +1068,15 @@ out: * the process which keeps the master peer. */ if (root_item->sid != vpid(root_item)) { - pr_debug("Restore inherited group %d\n", - getpgid(getppid())); - if (tty_set_prgp(fd, getpgid(getppid()))) - goto err; + if (root_item->pgid == vpid(root_item)) { + if (tty_set_prgp(fd, root_item->pgid)) + goto err; + } else { + pr_debug("Restore inherited group %d\n", + getpgid(getppid())); + if (tty_set_prgp(fd, getpgid(getppid()))) + goto err; + } } }
On Fri, Jan 18, 2019 at 12:43:14AM +0000, Radostin Stoyanov wrote: > The current behaviour of CRIU is to inherit the group leader of the > parent process when migrating a session leader (with --shell-job). > However, it is possible for a process to be group leader without being > a session leader (e.g. a shell process). In this case CRIU should > restore the original group ID of the process and inherit only the > session ID. > > Closes #593 > > Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com> Good catch! Will try to review more carefully today.
On Fri, Jan 18, 2019 at 12:43:14AM +0000, Radostin Stoyanov wrote: > The current behaviour of CRIU is to inherit the group leader of the > parent process when migrating a session leader (with --shell-job). > However, it is possible for a process to be group leader without being > a session leader (e.g. a shell process). In this case CRIU should > restore the original group ID of the process and inherit only the > session ID. > > Closes #593 > > Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com> Reviewed-by: Cyrill Gorcunov <gorcunov@gmail.com>
Applied, thanks! On Fri, Jan 18, 2019 at 12:43:14AM +0000, Radostin Stoyanov wrote: > The current behaviour of CRIU is to inherit the group leader of the > parent process when migrating a session leader (with --shell-job). > However, it is possible for a process to be group leader without being > a session leader (e.g. a shell process). In this case CRIU should > restore the original group ID of the process and inherit only the > session ID. > > Closes #593 > > Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com> > --- > criu/pstree.c | 27 ++++++++++++++++++--------- > criu/tty.c | 13 +++++++++---- > 2 files changed, 27 insertions(+), 13 deletions(-) > > diff --git a/criu/pstree.c b/criu/pstree.c > index 695110c45..92b4167aa 100644 > --- a/criu/pstree.c > +++ b/criu/pstree.c > @@ -367,22 +367,31 @@ static int prepare_pstree_for_shell_job(pid_t pid) > */ > > old_sid = root_item->sid; > - old_gid = root_item->pgid; > > - pr_info("Migrating process tree (GID %d->%d SID %d->%d)\n", > - old_gid, current_gid, old_sid, current_sid); > + pr_info("Migrating process tree (SID %d->%d)\n", > + old_sid, current_sid); > > for_each_pstree_item(pi) { > - if (pi->pgid == old_gid) > - pi->pgid = current_gid; > if (pi->sid == old_sid) > pi->sid = current_sid; > } > > - if (lookup_create_item(current_sid) == NULL) > - return -1; > - if (lookup_create_item(current_gid) == NULL) > - return -1; > + old_gid = root_item->pgid; > + if (old_gid != vpid(root_item)) { > + if (lookup_create_item(current_sid) == NULL) > + return -1; > + > + pr_info("Migrating process tree (GID %d->%d)\n", > + old_gid, current_gid); > + > + for_each_pstree_item(pi) { > + if (pi->pgid == old_gid) > + pi->pgid = current_gid; > + } > + > + if (lookup_create_item(current_gid) == NULL) > + return -1; > + } > > return 0; > } > diff --git a/criu/tty.c b/criu/tty.c > index 38e1cab33..5aaca633a 100644 > --- a/criu/tty.c > +++ b/criu/tty.c > @@ -1068,10 +1068,15 @@ out: > * the process which keeps the master peer. > */ > if (root_item->sid != vpid(root_item)) { > - pr_debug("Restore inherited group %d\n", > - getpgid(getppid())); > - if (tty_set_prgp(fd, getpgid(getppid()))) > - goto err; > + if (root_item->pgid == vpid(root_item)) { > + if (tty_set_prgp(fd, root_item->pgid)) > + goto err; > + } else { > + pr_debug("Restore inherited group %d\n", > + getpgid(getppid())); > + if (tty_set_prgp(fd, getpgid(getppid()))) > + goto err; > + } > } > } > > -- > 2.20.1 > > _______________________________________________ > CRIU mailing list > CRIU@openvz.org > https://lists.openvz.org/mailman/listinfo/criu
The current behaviour of CRIU is to inherit the group leader of the parent process when migrating a session leader (with --shell-job). However, it is possible for a process to be group leader without being a session leader (e.g. a shell process). In this case CRIU should restore the original group ID of the process and inherit only the session ID. Closes #593 Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com> --- criu/pstree.c | 27 ++++++++++++++++++--------- criu/tty.c | 13 +++++++++---- 2 files changed, 27 insertions(+), 13 deletions(-) -- 2.20.1