From f502cfdd9b4c464af9ab73c00177d6e42621be25 Mon Sep 17 00:00:00 2001 From: Adam Williamson Date: Sun, 10 Jul 2016 06:43:30 -0700 Subject: [PATCH] backport PR #534 to fix #535 and openQA #781 --- 534.patch | 376 +++++++++++++++++++++++++++++++++++++++++++++++ os-autoinst.spec | 9 +- 2 files changed, 383 insertions(+), 2 deletions(-) create mode 100644 534.patch diff --git a/534.patch b/534.patch new file mode 100644 index 0000000..2518db7 --- /dev/null +++ b/534.patch @@ -0,0 +1,376 @@ +From 9f3ee6aa451470ca258e5452171c801b096ef7d2 Mon Sep 17 00:00:00 2001 +From: Stephan Kulow +Date: Sat, 9 Jul 2016 17:54:29 +0200 +Subject: [PATCH 1/2] Rework how exit status of isotovideo is calculated + +The signalling between the processes needs more care +--- + autotest.pm | 6 +++++ + backend/baseclass.pm | 2 +- + backend/driver.pm | 4 +--- + commands.pm | 5 ++++ + isotovideo | 68 ++++++++++++++++++++++++++++++++++++---------------- + myjsonrpc.pm | 11 +++++++-- + 6 files changed, 70 insertions(+), 26 deletions(-) + +diff --git a/autotest.pm b/autotest.pm +index 6acc443..766bd42 100644 +--- a/autotest.pm ++++ b/autotest.pm +@@ -116,6 +116,7 @@ sub run_all { + warn $@; + $r = 1; + } ++ myjsonrpc::send_json($isotovideo, {cmd => 'tests_done', ret => $r}); + close $isotovideo; + _exit(0); + } +@@ -138,6 +139,11 @@ sub start_process { + die "cannot fork: $!" unless defined $testpid; + close $child; + ++ $SIG{TERM} = 'DEFAULT'; ++ $SIG{INT} = 'DEFAULT'; ++ $SIG{HUP} = 'DEFAULT'; ++ $SIG{CHLD} = 'DEFAULT'; ++ + $0 = "$0: autotest"; + my $line = <$isotovideo>; + if (!$line) { +diff --git a/backend/baseclass.pm b/backend/baseclass.pm +index 87ad721..79deb47 100644 +--- a/backend/baseclass.pm ++++ b/backend/baseclass.pm +@@ -271,7 +271,7 @@ sub alive { + } + else { + bmwqemu::diag("ALARM: backend.run got deleted! - exiting..."); +- alarm 3; ++ _exit(1); + } + } + return 0; +diff --git a/backend/driver.pm b/backend/driver.pm +index c9d620e..5ed377f 100644 +--- a/backend/driver.pm ++++ b/backend/driver.pm +@@ -70,13 +70,11 @@ sub start { + die "fork failed" unless defined $pid; + + if ($pid == 0) { +- $SIG{ALRM} = 'DEFAULT'; + $SIG{TERM} = 'DEFAULT'; + $SIG{INT} = 'DEFAULT'; + $SIG{HUP} = 'DEFAULT'; + $SIG{CHLD} = 'DEFAULT'; +- +- $0 = "$0: backend"; ++ $0 = "$0: backend"; + $self->{backend}->run(fileno($self->{from_parent}), fileno($self->{to_parent})); + _exit(0); + } +diff --git a/commands.pm b/commands.pm +index e602735..3562fdf 100644 +--- a/commands.pm ++++ b/commands.pm +@@ -281,6 +281,11 @@ sub start_server { + die "fork failed" unless defined $pid; + + if ($pid == 0) { ++ $SIG{TERM} = 'DEFAULT'; ++ $SIG{INT} = 'DEFAULT'; ++ $SIG{HUP} = 'DEFAULT'; ++ $SIG{CHLD} = 'DEFAULT'; ++ + close($child); + $0 = "$0: commands"; + run_daemon($port); +diff --git a/isotovideo b/isotovideo +index e76f806..167730c 100755 +--- a/isotovideo ++++ b/isotovideo +@@ -45,6 +45,7 @@ use testapi qw(diag); + use Getopt::Std; + require IPC::System::Simple; + use autodie qw(:all); ++no autodie qw(kill); + use Cwd; + use POSIX qw(:sys_wait_h _exit); + use Carp qw(cluck); +@@ -80,24 +81,30 @@ die "No scripts in $bmwqemu::vars{CASEDIR}" if !-e "$bmwqemu::vars{CASEDIR}"; + my $cpid; + my $cfd; + ++my $loop = 1; ++ + # all so ugly ... + sub signalhandler { + + my ($sig) = @_; +- diag("$$: signalhandler got $sig"); ++ diag("signalhandler got $sig - loop $loop"); ++ if ($loop) { ++ $loop = 0; ++ return; ++ } + bmwqemu::stop_vm(); + if (defined $bmwqemu::backend && $bmwqemu::backend->{backend_pid}) { +- diag("$$: killing backend process $bmwqemu::backend->{backend_pid}"); ++ diag("killing backend process $bmwqemu::backend->{backend_pid}"); + kill('SIGTERM', $bmwqemu::backend->{backend_pid}); + waitpid($bmwqemu::backend->{backend_pid}, 0); +- diag("$$: done with backend process"); ++ diag("done with backend process"); + $bmwqemu::backend->{backend_pid} = 0; + } + if (defined $cpid) { +- diag("$$: killing commands process $cpid"); ++ diag("killing commands process $cpid"); + kill('SIGTERM', $cpid); + waitpid($cpid, 0); +- diag("$$: done joining commands process"); ++ diag("done joining commands process"); + $cpid = 0; + } + _exit(1); +@@ -105,19 +112,23 @@ sub signalhandler { + + sub signalhandler_chld { + +- diag("$$: got sigchld"); ++ diag("got sigchld"); + + while ((my $child = waitpid(-1, WNOHANG)) > 0) { + if ($child == $cpid) { +- diag("$$: commands webserver died"); ++ diag("commands webserver died"); ++ $loop = 0; + $cpid = 0; ++ next; + } + if ($bmwqemu::backend->{backend_pid} && $child == $bmwqemu::backend->{backend_pid}) { +- diag("$$: backend $child died"); ++ diag("backend $child died"); + $bmwqemu::backend->{backend_pid} = 0; + $bmwqemu::backend->{to_child} = undef; + $bmwqemu::backend->{from_child} = undef; ++ next; + } ++ diag("unknown child $child died"); + } + } + +@@ -142,7 +153,6 @@ sub calculate_git_hash { + return $test_git_hash; + } + +-$SIG{ALRM} = \&signalhandler; + $SIG{TERM} = \&signalhandler; + $SIG{INT} = \&signalhandler; + $SIG{HUP} = \&signalhandler; +@@ -215,8 +225,6 @@ my $needinput = 0; + + my $current_test_name; + +-my $loop = 1; +- + # timeout for the select (only set for check_screens) + my $timeout = undef; + +@@ -263,6 +271,8 @@ sub check_asserted_screen { + } + } + ++my $r = 0; ++ + while ($loop) { + my ($reads, $writes, $exceps) = IO::Select::select($s, undef, $s, $timeout); + for my $r (@$reads) { +@@ -291,6 +301,17 @@ while ($loop) { + myjsonrpc::send_json($r, {ret => 1}); + next; + } ++ if ($rsp->{cmd} eq 'tests_done') { ++ $r = $rsp->{ret}; ++ close($testfd); ++ $testfd = undef; ++ kill('SIGTERM', $testpid); ++ diag "awaiting death of test process $testpid"; ++ my $ret = waitpid($testpid, 0); ++ diag "test process $testpid exited"; ++ $loop = 0; ++ next; ++ } + if ($rsp->{cmd} eq 'check_screen') { + my $mustmatch = $rsp->{mustmatch}; + my $timeout = $rsp->{timeout}; +@@ -358,28 +379,33 @@ while ($loop) { + check_asserted_screen; + } + } +-close $testfd; +-kill('SIGTERM', $testpid); +-diag "awaiting death of testpid $testpid"; +-waitpid($testpid, 0); + +-my $r = 0; ++if ($testfd) { ++ $r = 1; # unusual shutdown ++ close $testfd; ++ kill('SIGTERM', $testpid); ++ diag "awaiting death of testpid $testpid"; ++ my $ret = waitpid($testpid, 0); ++ diag "test process exited: $ret"; ++} ++ + diag "isotovideo done" unless $r; + diag "FAIL" if $r; + + my $clean_shutdown; + eval { +- my $status = $bmwqemu::backend->status(); ++ my $status = $bmwqemu::backend->_send_json({cmd => 'status'}); ++ diag "BACKEND STATUS $status\n"; + $clean_shutdown = 1 if $status || '' eq "shutdown"; + }; + + bmwqemu::stop_vm(); +-diag "killing commands process"; + if ($cpid) { ++ diag "killing commands process"; + kill('SIGTERM', $cpid); +- waitpid($cpid, 0); ++ my $ret = waitpid($cpid, 0); ++ diag "commands process exited: $ret"; + } +-diag "done joining commands process"; + + # mark hard disks for upload if test finished + if (!$r && (my $nd = $bmwqemu::vars{NUMDISKS})) { +@@ -400,6 +426,7 @@ if (!$r && (my $nd = $bmwqemu::vars{NUMDISKS})) { + } + if (@toextract && !$clean_shutdown) { + diag "ERROR: Machine not shut down when uploading disks!\n"; ++ $r = 1; + } + else { + for my $asset (@toextract) { +@@ -408,5 +435,6 @@ if (!$r && (my $nd = $bmwqemu::vars{NUMDISKS})) { + } + } + ++print "EXIT $r\n"; + exit $r; + # vim: set sw=4 et: +diff --git a/myjsonrpc.pm b/myjsonrpc.pm +index 505ea5f..374574a 100644 +--- a/myjsonrpc.pm ++++ b/myjsonrpc.pm +@@ -19,6 +19,7 @@ use strict; + use warnings; + use Carp qw(cluck confess); + use bmwqemu (); ++use Errno; + + sub send_json { + my ($to_fd, $cmd) = @_; +@@ -64,7 +65,7 @@ sub read_json { + # remember the trailing text + $sockets->{$fd} = $JSON->incr_text(); + if ($hash->{QUIT}) { +- print "received magic close\n"; ++ bmwqemu::diag("received magic close"); + return; + } + return $hash; +@@ -74,7 +75,13 @@ sub read_json { + my @res = $s->can_read; + unless (@res) { + my $E = $!; # save the error +- confess "ERROR: timeout reading JSON reply: $E\n"; ++ unless ($!{EINTR}) { # EINTR if killed ++ confess "ERROR: timeout reading JSON reply: $E\n"; ++ } ++ else { ++ bmwqemu::diag("can_read received kill signal"); ++ } ++ return; + } + + my $qbuffer; + +From 4485a511c5e0840eccca21c5be659c2f309b9cd9 Mon Sep 17 00:00:00 2001 +From: Stephan Kulow +Date: Sun, 10 Jul 2016 11:59:00 +0200 +Subject: [PATCH 2/2] Make sure the opencv threads only run in the backend + +That opencv creates threads in our back confuses PERL when delivering +signals, so we need to double catch the problem by only initiing +cv in the forked children and avoiding IPC::Simple (via autodie) +in isotovideo +--- + autotest.pm | 4 ++++ + backend/qemu.pm | 3 ++- + bmwqemu.pm | 6 ------ + 3 files changed, 6 insertions(+), 7 deletions(-) + +diff --git a/autotest.pm b/autotest.pm +index 766bd42..c90844b 100644 +--- a/autotest.pm ++++ b/autotest.pm +@@ -25,6 +25,7 @@ use File::Spec; + use Socket; + use IO::Handle; + use POSIX qw(_exit); ++use cv; + + our %tests; # scheduled or run tests + our @testorder; # for keeping them in order +@@ -144,6 +145,9 @@ sub start_process { + $SIG{HUP} = 'DEFAULT'; + $SIG{CHLD} = 'DEFAULT'; + ++ cv::init; ++ require tinycv; ++ + $0 = "$0: autotest"; + my $line = <$isotovideo>; + if (!$line) { +diff --git a/backend/qemu.pm b/backend/qemu.pm +index 869ccbd..71b9203 100644 +--- a/backend/qemu.pm ++++ b/backend/qemu.pm +@@ -173,7 +173,8 @@ sub load_snapshot { + + sub runcmd { + diag "running " . join(' ', @_); +- return system(@_); ++ local $SIG{CHLD} = 'IGNORE'; ++ return CORE::system(@_); + } + + sub do_extract_assets { +diff --git a/bmwqemu.pm b/bmwqemu.pm +index a02e540..75109b5 100755 +--- a/bmwqemu.pm ++++ b/bmwqemu.pm +@@ -20,9 +20,6 @@ use warnings; + use Time::HiRes qw(sleep gettimeofday); + use IO::Socket; + +-use ocr; +-use cv; +-use needle; + use Thread::Queue; + use POSIX; + use Term::ANSIColor; +@@ -125,9 +122,6 @@ sub init { + $| = 1; + select($oldfh); + +- cv::init(); +- require tinycv; +- + die "DISTRI undefined\n" . pp(\%vars) . "\n" unless $vars{DISTRI}; + + unless ($vars{CASEDIR}) { diff --git a/os-autoinst.spec b/os-autoinst.spec index 8e76c1a..59bbf41 100644 --- a/os-autoinst.spec +++ b/os-autoinst.spec @@ -31,12 +31,14 @@ Name: os-autoinst Version: %{github_version} -Release: 16%{?github_date:.%{github_date}git%{shortcommit}}%{?dist} +Release: 17%{?github_date:.%{github_date}git%{shortcommit}}%{?dist} Summary: OS-level test automation License: GPLv2+ Group: Development/System URL: https://os-autoinst.github.io/openQA/ Source0: https://github.com/%{github_owner}/%{github_name}/archive/%{github_commit}/%{github_name}-%{github_commit}.tar.gz +# Fix some test teardown bugs, inc. GH#535 and openQA GH#781 +Patch0: https://github.com/os-autoinst/os-autoinst/pull/534.patch BuildRequires: autoconf BuildRequires: automake @@ -191,7 +193,10 @@ make check VERBOSE=1 %config(noreplace) %{_sysconfdir}/dbus-1/system.d/org.opensuse.os_autoinst.switch.conf %changelog -* Fri Jul 08 2016 Adam Williamson - 4.3-16.20160624git7a1901d +* Sun Jul 10 2016 Adam Williamson - 4.3.17.20160708git7a1901d +- backport PR #534 to fix #535 and openQA #781 + +* Fri Jul 08 2016 Adam Williamson - 4.3-16.20160708git7a1901d - bump to latest git - drop merged PR #524 patch