[kvm-unit-tests PATCH v2] runtime: better handling of QEMU aborts

[kvm-unit-tests PATCH v2] runtime: better handling of QEMU aborts

[Andrew Jones]

Add two changes to run_qemu. The first saves/restores terminal settings.
This solves an annoying loss of terminal echo when QEMU aborts during
a test run. The second ensures we see a message about the abort, because
the "Aborted (core dumped)" message we should see gets eaten. We also
add a message to run()'s failure cases in its exit code processing to
handle signals in general.

Note, the first change is necessary because QEMU modifies the terminal
settings when using '-serial stdio', but calling abort() invokes exit
without first calling qemu_chr_free(serial_hds[0]) to restore them.

(Additionally we fixup the premature failure check to only capture the
 last line, like it says it's doing.)

Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Andrew Jones <drjones@redhat.com>

---
v2:
 - rebase to latest master and add FAIL case to run()'s exit code
   processing [Radim]
 - add '>(tail -1) to premature failure check [Radim]
---
 scripts/arch-run.bash | 5 ++++-
 scripts/runtime.bash  | 4 +++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index 4b6f1aedd62c..9770872f1130 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -26,13 +26,16 @@
 ##############################################################################
 run_qemu ()
 {
-	local stdout errors ret sig
+	local stdout errors ret sig tty
 
 	# stdout to {stdout}, stderr to $errors and stderr
+	tty=$(stty -g)
 	exec {stdout}>&1
 	errors=$("${@}" 2> >(tee /dev/stderr) > /dev/fd/$stdout)
 	ret=$?
+	[ $ret -eq 134 ] && echo "QEMU Aborted" > /dev/fd/$stdout
 	exec {stdout}>&-
+	stty "$tty"
 
 	if [ "$errors" ]; then
 		sig=$(grep 'terminating on signal' <<<"$errors")
diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index 1bd87bcdf17a..11a40a92183a 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -92,7 +92,7 @@ function run()
         fi
     done
 
-    last_line=$(premature_failure) && {
+    last_line=$(premature_failure > >(tail -1)) && {
         echo "`SKIP` $1 ($last_line)"
         return 77
     }
@@ -115,6 +115,8 @@ function run()
         echo "`SKIP` $1 $summary"
     elif [ $ret -eq 124 ]; then
         echo "`FAIL` $1 (timeout; duration=$timeout)"
+    elif [ $ret -gt 127 ]; then
+        echo "`FAIL` $1 (terminated on SIG$(kill -l $(($ret - 128))))"
     else
         echo "`FAIL` $1 $summary"
     fi
-- 
2.7.4

Re: [kvm-unit-tests PATCH v2] runtime: better handling of QEMU aborts

[Radim Krčmář]

2016-11-15 18:26+0100, Andrew Jones:
> Add two changes to run_qemu. The first saves/restores terminal settings.
> This solves an annoying loss of terminal echo when QEMU aborts during
> a test run. The second ensures we see a message about the abort, because
> the "Aborted (core dumped)" message we should see gets eaten. We also
> add a message to run()'s failure cases in its exit code processing to
> handle signals in general.
> 
> Note, the first change is necessary because QEMU modifies the terminal
> settings when using '-serial stdio', but calling abort() invokes exit
> without first calling qemu_chr_free(serial_hds[0]) to restore them.
> 
> (Additionally we fixup the premature failure check to only capture the
>  last line, like it says it's doing.)
> 
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> 
> ---
> v2:
>  - rebase to latest master and add FAIL case to run()'s exit code
>    processing [Radim]
>  - add '>(tail -1) to premature failure check [Radim]
> ---
> diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> @@ -26,13 +26,16 @@
>  ##############################################################################
>  run_qemu ()
>  {
> -	local stdout errors ret sig
> +	local stdout errors ret sig tty
>  
>  	# stdout to {stdout}, stderr to $errors and stderr
> +	tty=$(stty -g)
>  	exec {stdout}>&1
>  	errors=$("${@}" 2> >(tee /dev/stderr) > /dev/fd/$stdout)
>  	ret=$?
> +	[ $ret -eq 134 ] && echo "QEMU Aborted" > /dev/fd/$stdout

Standard output is /dev/fd/$stdout here (unlike in parentheses, where it
was $errors).  I can remove the redirection when applying, but
redirecting to stderr would also make sense -- what do you prefer?

>  	exec {stdout}>&-
> +	stty "$tty"
>  
>  	if [ "$errors" ]; then
>  		sig=$(grep 'terminating on signal' <<<"$errors")

Re: [kvm-unit-tests PATCH v2] runtime: better handling of QEMU aborts

[Andrew Jones]

On Wed, Nov 16, 2016 at 09:47:29PM +0100, Radim Krčmář wrote:
> 2016-11-15 18:26+0100, Andrew Jones:
> > Add two changes to run_qemu. The first saves/restores terminal settings.
> > This solves an annoying loss of terminal echo when QEMU aborts during
> > a test run. The second ensures we see a message about the abort, because
> > the "Aborted (core dumped)" message we should see gets eaten. We also
> > add a message to run()'s failure cases in its exit code processing to
> > handle signals in general.
> > 
> > Note, the first change is necessary because QEMU modifies the terminal
> > settings when using '-serial stdio', but calling abort() invokes exit
> > without first calling qemu_chr_free(serial_hds[0]) to restore them.
> > 
> > (Additionally we fixup the premature failure check to only capture the
> >  last line, like it says it's doing.)
> > 
> > Reported-by: Peter Maydell <peter.maydell@linaro.org>
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > 
> > ---
> > v2:
> >  - rebase to latest master and add FAIL case to run()'s exit code
> >    processing [Radim]
> >  - add '>(tail -1) to premature failure check [Radim]
> > ---
> > diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> > @@ -26,13 +26,16 @@
> >  ##############################################################################
> >  run_qemu ()
> >  {
> > -	local stdout errors ret sig
> > +	local stdout errors ret sig tty
> >  
> >  	# stdout to {stdout}, stderr to $errors and stderr
> > +	tty=$(stty -g)
> >  	exec {stdout}>&1
> >  	errors=$("${@}" 2> >(tee /dev/stderr) > /dev/fd/$stdout)
> >  	ret=$?
> > +	[ $ret -eq 134 ] && echo "QEMU Aborted" > /dev/fd/$stdout
> 
> Standard output is /dev/fd/$stdout here (unlike in parentheses, where it
> was $errors).  I can remove the redirection when applying, but
> redirecting to stderr would also make sense -- what do you prefer?

stderr works for me. Thanks for the fixups

drew

> 
> >  	exec {stdout}>&-
> > +	stty "$tty"
> >  
> >  	if [ "$errors" ]; then
> >  		sig=$(grep 'terminating on signal' <<<"$errors")
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [kvm-unit-tests PATCH v2] runtime: better handling of QEMU aborts

[Radim Krčmář]

2016-11-17 18:06+0100, Andrew Jones:
> On Wed, Nov 16, 2016 at 09:47:29PM +0100, Radim Krčmář wrote:
>> 2016-11-15 18:26+0100, Andrew Jones:
>> > Add two changes to run_qemu. The first saves/restores terminal settings.
>> > This solves an annoying loss of terminal echo when QEMU aborts during
>> > a test run. The second ensures we see a message about the abort, because
>> > the "Aborted (core dumped)" message we should see gets eaten. We also
>> > add a message to run()'s failure cases in its exit code processing to
>> > handle signals in general.
>> > 
>> > Note, the first change is necessary because QEMU modifies the terminal
>> > settings when using '-serial stdio', but calling abort() invokes exit
>> > without first calling qemu_chr_free(serial_hds[0]) to restore them.
>> > 
>> > (Additionally we fixup the premature failure check to only capture the
>> >  last line, like it says it's doing.)
>> > 
>> > Reported-by: Peter Maydell <peter.maydell@linaro.org>
>> > Signed-off-by: Andrew Jones <drjones@redhat.com>
>> > 
>> > ---
>> > v2:
>> >  - rebase to latest master and add FAIL case to run()'s exit code
>> >    processing [Radim]
>> >  - add '>(tail -1) to premature failure check [Radim]
>> > ---
>> > diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
>> > @@ -26,13 +26,16 @@
>> >  ##############################################################################
>> >  run_qemu ()
>> >  {
>> > -	local stdout errors ret sig
>> > +	local stdout errors ret sig tty
>> >  
>> >  	# stdout to {stdout}, stderr to $errors and stderr
>> > +	tty=$(stty -g)
>> >  	exec {stdout}>&1
>> >  	errors=$("${@}" 2> >(tee /dev/stderr) > /dev/fd/$stdout)
>> >  	ret=$?
>> > +	[ $ret -eq 134 ] && echo "QEMU Aborted" > /dev/fd/$stdout
>> 
>> Standard output is /dev/fd/$stdout here (unlike in parentheses, where it
>> was $errors).  I can remove the redirection when applying, but
>> redirecting to stderr would also make sense -- what do you prefer?
> 
> stderr works for me. Thanks for the fixups

Done that and while at it, I also moved it a bit down, for clarity.