[CRIU] [PATCH 2/3] test: app-emu -- job: Fix comparison

[Andrew Vagin]

On Tue, May 07, 2013 at 12:23:35PM +0400, Cyrill Gorcunov wrote:
> On Tue, May 07, 2013 at 12:01:43PM +0400, Andrew Vagin wrote:
> > On Mon, May 06, 2013 at 11:34:32PM +0400, Cyrill Gorcunov wrote:
> > > On Mon, May 06, 2013 at 11:18:57PM +0400, Andrey Wagin wrote:
> > > >    On May 6, 2013 7:13 PM, "Cyrill Gorcunov" <gorcunov at openvz.org> wrote:
> > > >    >
> > > >    > Since we're migrating session leader here,
> > > >    > test for session has not been changed instead.
> > > > 
> > > >    In this case the test doesn't wait c/r. I don't like when sleep is used
> > > >    for synchronization instead of sending a signal.
> > > 
> > > It does wait. Maybe in somehow weird way (ie by sleeps) but still. The main
> > > expect script writes pid
> > > 
> > > 	system ../../../crtools dump -v 4 -D ./dump -o dump.log -j -t $pid
> > > 	system echo "$pid" > ./dump/pid.pid
> > > 
> > > once dump complete, then restore stage initiated.
> > 
> > Look at the attached patch.
> 
> Will you be fine with this one?
> ---
> From: Andrew Vagin <avagin at parallels.com>
> Subject: [PATCH] test: app-emu -- job: Fix comparison and wait for restore complete by a signal
> 
> Since we're migrating session leader here, test for session has not
> been changed instead.
> 
> [gorcunov@: Add small sleep into expect script]
> 
> Signed-off-by: Andrew Vagin <avagin at parallels.com>
> Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
> ---
>  test/app-emu/job/job.c   | 36 +++++++++++++++++++++++-------------
>  test/app-emu/job/job.exp |  6 ++++++
>  2 files changed, 29 insertions(+), 13 deletions(-)
> 
> diff --git a/test/app-emu/job/job.c b/test/app-emu/job/job.c
> index e01480a..44858a7 100644
> --- a/test/app-emu/job/job.c
> +++ b/test/app-emu/job/job.c
> @@ -14,15 +14,27 @@
>  
>  #include <dirent.h>
>  
> +static int stop = 0;
> +
> +void sighandler(int sig)
> +{
> +	stop = 1;
> +}
> +
>  int main(int argc, char *argv[])
>  {
>  	int pid, gid, sid;
>  	int tty_sid, tty_gid;
>  	int fd = fileno(stdout);
>  	char buf[32];
> -	int c = 0;
>  	struct dirent *de;
>  	DIR *fd_dir;
> +	sigset_t bmask, cmask;
> +
> +	if (signal(SIGTERM, sighandler)) {
> +		printf("Unable to set a signal handler: %m\n");
> +		return 1;
> +	}
>  
>  	if (!isatty(fd)) {
>  		printf("stdout is not tty\n");
> @@ -70,21 +82,19 @@ int main(int argc, char *argv[])
>  	}
>  	printf("stdout gid = %d\n", tty_gid);
>  
> +	sigemptyset(&cmask);
> +	sigemptyset(&bmask);
> +	sigaddset(&bmask, SIGTERM);
> +
> +	sigprocmask(SIG_SETMASK, &bmask, NULL);
> +
>  	printf("READY\n");
>  
> -	c = 0;
> -	while (1) {
> -		sleep(1);
> -		if (c++ > 10) {
> -			printf("Too long for restore\n");
> -			exit(-1);
> -		}
> +	while (!stop)
> +		sigsuspend(&cmask);
>  
> -		if (getsid(pid) != sid) {
> -			printf("ALIVE\n");
> -			break;
> -		}
> -	}
> +	if (getsid(pid) == sid)
> +		printf("ALIVE\n");
>  
>  	return 0;
>  }
> diff --git a/test/app-emu/job/job.exp b/test/app-emu/job/job.exp
> index cf743e8..1b4dcbd 100755
> --- a/test/app-emu/job/job.exp
> +++ b/test/app-emu/job/job.exp
> @@ -38,6 +38,12 @@ switch $current {
>  		}
>  
>  		spawn ../../../crtools restore -v 4 -D ./dump -o restore.log -j -t $pid
> +		#
> +		# spawn doesn't wait for restore to complete, so
> +		# add some sleep here. Still better would be to
> +		# rewrite this test completely.
> +		sleep 2

Eh. Pls, commit this patch without my ack. I don't want to be a part of
this. Sorry.

> +		system kill -15 $pid
>  
>  		expect "ALIVE" {
>  			puts "PASS"
> -- 
> 1.8.1.4
>