[CRIU] [PATCH] zdtm: Fix fd01 cleanup

[Andrei Vagin]

On Thu, Feb 01, 2018 at 05:48:22PM +0300, Kirill Tkhai wrote:
> On 31.01.2018 21:20, Andrei Vagin wrote:
> > On Tue, Jan 30, 2018 at 06:19:31PM +0300, Kirill Tkhai wrote:
> >> waitpid() does not return child pid, when child has not exited.
> >> So, we can't use it to find pids of children.
> >>
> >> Signed-off-by: Kirill Tkhai <ktkhai at virtuozzo.com>
> >> ---
> >>  test/zdtm/static/fd01.c |   17 ++++++++++++-----
> >>  1 file changed, 12 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/test/zdtm/static/fd01.c b/test/zdtm/static/fd01.c
> >> index 7f0256a03..390d9d126 100644
> >> --- a/test/zdtm/static/fd01.c
> >> +++ b/test/zdtm/static/fd01.c
> >> @@ -22,11 +22,19 @@ int main(int argc, char **argv)
> >>  	unsigned int i, max_nr, flags;
> >>  	int fd, status, ret;
> >>  	struct rlimit rlim;
> >> +	futex_t *futex;
> >>  	char buf[16];
> >>  	pid_t pid;
> >>  
> >>  	test_init(argc, argv);
> >>  
> >> +	futex = mmap(NULL, sizeof(*futex), PROT_WRITE | PROT_READ, MAP_ANONYMOUS|MAP_SHARED, -1, 0);
> >> +	if (futex == MAP_FAILED) {
> >> +		fail("mmap");
> >> +		exit(1);
> >> +	}
> >> +	futex_init(futex);
> >> +
> >>  	fd = open("/proc/sys/fs/nr_open", O_RDONLY);
> >>  	if (fd < 0) {
> >>  		fail("Can't open /proc/sys/fs/nr_open");
> >> @@ -86,7 +94,7 @@ int main(int argc, char **argv)
> >>  			fail("fork");
> >>  			exit(1);
> >>  		} else if (!pid) {
> >> -			pause();
> >> +			futex_wait_while(futex, 0);
> >>  			exit(0);
> >>  		}
> >>  	}
> >> @@ -95,10 +103,9 @@ int main(int argc, char **argv)
> >>  	test_waitsig();
> >>  
> >>  	/* Cleanup */
> >> -	while ((pid = waitpid(-1, &status, WNOHANG)) > 0) {
> >> -		if (kill(pid, SIGTERM) == 0)
> >> -			waitpid(-1, &status, 0); /* Ignore errors */
> >> -	}
> >> +	futex_set_and_wake(futex, 1);
> >> +	while (wait(&status) > 0) /* Ignore errors */
> >> +		;
> > 
> > Pls, check an exit code from children, otherwise it isn't a test,
> > because it nevel fails.
> 
> Ok, no problem. But the idea was to use zdtm.py debug hooks, which checks
> for file descriptors in check_visible_state()...:
> 
>     print "%s: Old files lost: %s" % (pid, fold - fnew)
>     print "%s: New files appeared: %s" % (pid, fnew - fold)
> 
> >In addition, it would be good if you add
> >basic checks to be sure that file descriptors are restored correctly.
> 
> Sadly, but it won't be possible to check that in sane way, because
> the behaviour of created children and files is almost "random".
> I don't see a good way to check everything in the test having so many
> entities. Maybe we better implement one more? And remain this test
> for checks I wrote above.

You are right, the zdtm hook does some basic checks, so I think it is
enough. Thank you.


> 
> Kirill