From mboxrd@z Thu Jan 1 00:00:00 1970 From: Radim =?utf-8?B?S3LEjW3DocWZ?= Subject: Re: [kvm-unit-tests PATCH v2] runtime: better handling of QEMU aborts Date: Tue, 22 Nov 2016 14:58:33 +0100 Message-ID: <20161122135832.GE12949@potion> References: <1479230784-5640-1-git-send-email-drjones@redhat.com> <20161116204728.GC20282@potion> <20161117170656.mokgfuixb7ncktww@hawk.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: kvm@vger.kernel.org, pbonzini@redhat.com, peter.maydell@linaro.org To: Andrew Jones Return-path: Received: from mx1.redhat.com ([209.132.183.28]:57766 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932934AbcKVN6g (ORCPT ); Tue, 22 Nov 2016 08:58:36 -0500 Content-Disposition: inline In-Reply-To: <20161117170656.mokgfuixb7ncktww@hawk.localdomain> Sender: kvm-owner@vger.kernel.org List-ID: 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 >> > Signed-off-by: Andrew Jones >> > >> > --- >> > 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.