kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Radim Krčmář" <rkrcmar@redhat.com>
To: Andrew Jones <drjones@redhat.com>
Cc: kvm@vger.kernel.org, pbonzini@redhat.com
Subject: Re: [kvm-unit-tests PATCH 3/6] arch-run: reduce return code ambiguity
Date: Tue, 26 Jan 2016 22:31:56 +0100	[thread overview]
Message-ID: <20160126213155.GA1844@potion.brq.redhat.com> (raw)
In-Reply-To: <1453833731-23215-4-git-send-email-drjones@redhat.com>

2016-01-26 19:42+0100, Andrew Jones:
> qemu/unittest exit codes are convoluted, causing codes 0 and 1
> to be ambiguous. Here are the possible meanings
> 
>  .-----------------------------------------------------------------.
>  |                | 0                                 | 1          |
>  |-----------------------------------------------------------------|
>  | QEMU           | did something successfully,       | FAILURE    |
>  |                | but probably didn't run the       |            |
>  |                | unittest, OR caught SIGINT,       |            |
>  |                | SIGHUP, or SIGTERM                |            |
>  |-----------------------------------------------------------------|
>  | unittest       | for some reason exited using      | SUCCESS    |
>  |                | ACPI/PSCI, not with debug-exit    |            |
>  .-----------------------------------------------------------------.
> 
> As we can see above, an exit code of zero is even ambiguous for each
> row, i.e. QEMU could exit with zero because it successfully completed,
> or because it caught a signal. unittest could exit with zero because
> it successfully powered-off, or because for some odd reason it powered-
> off instead of calling debug-exit.
> 
> And, the most fun is that exit-code == 1 means QEMU failed, but the
> unittest succeeded.
> 
> This patch attempts to reduce that ambiguity, by also looking at stderr.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
> diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> @@ -0,0 +1,62 @@
> +##############################################################################
> +# exit_fixup translates the ambiguous exit status in Table1 to that in Table2.
> +# Table3 simply documents the complete status table.
> +#
> +# Table1: Before fixup
> +# --------------------
> +# 0      - Unexpected exit from QEMU (possible signal), or the unittest did
> +#          not use debug-exit
> +# 1      - most likely unittest succeeded, or QEMU failed
> +#
> +# Table2: After fixup
> +# -------------------
> +# 0      - Everything succeeded
> +# 1      - most likely QEMU failed
> +#
> +# Table3: Complete table
> +# ----------------------
> +# 0      - SUCCESS
> +# 1      - most likely QEMU failed
> +# 2      - most likely a run script failed
> +# 3      - most likely the unittest failed
> +# 127    - most likely the unittest called abort()
> +# 1..127 - FAILURE (could be QEMU, a run script, or the unittest)
> +# >= 128 - Signal (signum = status - 128)
> +##############################################################################
> +exit_fixup ()
> +{
> +	local stdout errors nr_errors ret sig
> +
> +	exec {stdout}>&1
> +	mapfile -t errors < <("${@}" 2>&1 1>&${stdout}; echo $?)
> +	exec {stdout}>&-
> +
> +	nr_errors=$((${#errors[@]} - 1))
> +	ret=${errors[$nr_errors]}
> +	unset errors[$nr_errors]

I think would be easier to understand as
  errors=$("${@}" 2>&1 1>&${stdout})
  ret=$?
  # using [ "$errors" ] instead of [ $nr_errors -ne 0 ]

> +
> +	if [ $nr_errors -ne 0 ]; then
> +		printf "%s\n" "${errors[@]}"

Should go to stderr.

(This reorders the output, so it would be a better to duplicate it when
 storing and print right away, but I only see inhumane designs for that.)

> +		sig=$(sed 's/.*terminating on signal \([0-9][0-9]*\).*/\1/' <<<"${errors[@]}")

If ${errors[@]} don't contain just 'terminating [...]' and QEMU returned
0, some text will be assigned to sig, which isn't going to be handled
well later on.

> +	fi
> +
> +	if [ $ret -eq 0 ]; then
> +		# Some signals result in a zero return status, but the
> +		# error log tells the truth.
> +		if [ "$sig" ]; then
> +			((ret=sig+128))
> +		else
> +			# Exiting with zero (non-debugexit) is an error
> +			ret=1
> +		fi
> +	elif [ $ret -eq 1 ]; then
> +		# Even when ret==1 (unittest success) if we also got stderr
> +		# logs, then we assume a QEMU failure. Otherwise we translate
> +		# status of 1 to 0 (SUCCESS)
> +		if [ $nr_errors -eq 0 ]; then
> +			ret=0
> +		fi
> +	fi
> +
> +	return $ret
> +}

  reply	other threads:[~2016-01-26 21:31 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-26 18:42 [kvm-unit-tests PATCH 0/6] reduce exit status ambiguity and more Andrew Jones
2016-01-26 18:42 ` [kvm-unit-tests PATCH 1/6] x86: clean up exit use, use abort Andrew Jones
2016-01-26 18:42 ` [kvm-unit-tests PATCH 2/6] run scripts need consistent exit status Andrew Jones
2016-01-26 18:42 ` [kvm-unit-tests PATCH 3/6] arch-run: reduce return code ambiguity Andrew Jones
2016-01-26 21:31   ` Radim Krčmář [this message]
2016-01-26 18:42 ` [kvm-unit-tests PATCH 4/6] cleanup unittests.cfg headers Andrew Jones
2016-01-26 18:42 ` [kvm-unit-tests PATCH 5/6] runtime: enable some unittest config overriding Andrew Jones
2016-01-26 21:36   ` Radim Krčmář
2016-01-26 18:42 ` [kvm-unit-tests PATCH 6/6] run scripts: add timeout support Andrew Jones
2016-01-26 21:44   ` Radim Krčmář
2016-01-26 21:55 ` [kvm-unit-tests PATCH 0/6] reduce exit status ambiguity and more Radim Krčmář
2016-01-27 10:33   ` Andrew Jones
2016-01-27 13:50     ` Radim Krčmář

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160126213155.GA1844@potion.brq.redhat.com \
    --to=rkrcmar@redhat.com \
    --cc=drjones@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).