[CRIU] test/others/rpc test cases

[Adrian Reber]

On Wed, May 23, 2018 at 02:26:38PM -0700, Andrei Vagin wrote:
> On Wed, May 16, 2018 at 05:16:12PM +0100, Dmitry Safonov wrote:
> > 2018-05-16 16:52 GMT+01:00 Adrian Reber <adrian at lisas.de>:
> > > Hello Andrei,
> > >
> > > we quickly chatted about the tests in test/others/rpc and I had a look
> > > at it.
> > >
> > > One test (errno.py) dumps itself, keeps on running and tries to restore
> > > itself. This should (and does fail) as the PID is already in use by the
> > > test case. The test case wants to to test the error handling of 'criu
> > > service'. But CRIU just kills all processes which it failed to restore
> > > in criu/cr-restore.c (around line 2240 : kill(vpid(pi), SIGKILL);).
> > >
> > > As the test case is killed by CRIU it counts as a failure. I guess this
> > > used to work at some point in time, but with 3.8.1 it does not.
> > >
> > > I am not sure what the right solution is. Should CRIU kill a process if
> > > restore fails, probably to make sure that no half restored process still
> > > exists. Maybe the test case does not make sense anymore...
> > >
> > > Do you have any thoughts on this? Is this a CRIU bug or a wrong test case?
> > 
> > It looks to me like a valid test. CRIU shouldn't kill existing tasks with
> > reused PID. Do we run this errno.py on Travis?
> > 
> > I haven't even compile tested, but something like this might fix?
> > 
> > --->8---
> > --- a/criu/cr-restore.c
> > +++ b/criu/cr-restore.c
> > @@ -1510,6 +1510,7 @@ static int do_fork_with_pid(struct pstree_item
> > *item, struct ns_id *pid_ns, stru
> >         if (pid < 0) {
> >                 pr_perror("Can't clone()");
> >                 ret = -1;
> > +               hlp_pid->ns[i].virt = -1;
> 
> If we have three processes A -> B -> C. PID-s of B, C have been reused.
> With you patch, virt for B will be set to -1, but a process with the C
> pid will be killed.
> 
> I think something like this should fix a problem:
> @@ -2556,8 +2556,8 @@ out_kill:
>                 struct pstree_item *pi;
>  
>                 for_each_pstree_item(pi)
> -                       if (vpid(pi) > 0)
> -                               kill(vpid(pi), SIGKILL);
> +                       if (pi->pid->real > 0)
> +                               kill(pi->pid->real, SIGKILL);
>         }
>  
>  out:
> 
> 
> >                 goto unblock;
> >         }

That patch fixes it, thanks. I will take your patch and submit it as
part of my series to re-enable the rpc test cases.

		Adrian