From mboxrd@z Thu Jan  1 00:00:00 1970
From: Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [kvm-unit-tests PATCH v3 3/6] arch-run: reduce return code
 ambiguity
Date: Tue, 1 Mar 2016 22:29:16 +0100
Message-ID: <56D609AC.80303@redhat.com>
References: <1456772003-27911-1-git-send-email-drjones@redhat.com>
 <1456772003-27911-4-git-send-email-drjones@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
Cc: rkrcmar@redhat.com
To: Andrew Jones <drjones@redhat.com>, kvm@vger.kernel.org
Return-path: <kvm-owner@vger.kernel.org>
Received: from mail-wm0-f68.google.com ([74.125.82.68]:33921 "EHLO
	mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1751933AbcCAV3T (ORCPT <rfc822;kvm@vger.kernel.org>);
	Tue, 1 Mar 2016 16:29:19 -0500
Received: by mail-wm0-f68.google.com with SMTP id p65so6326274wmp.1
        for <kvm@vger.kernel.org>; Tue, 01 Mar 2016 13:29:19 -0800 (PST)
In-Reply-To: <1456772003-27911-4-git-send-email-drjones@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>



On 29/02/2016 19:53, Andrew Jones wrote:
> +exit_fixup ()

Can you rename the function to run_qemu or something like that?
Otherwise it's not clear that it actually runs "$@".

> +	errors=$("${@}" 2>&1 1>&${stdout} | tee >(cat - 1>&2); exit ${PIPESTATUS[0]})

Whoa! :)  So stdout goes to {stdout} and from there to the real stdout;
stderr instead is tee'd to stderr and $errors.

Is the "tee" and "cat" necessary?  Can you just use printf somewhat like:

	# stdout to {stdout}, stderr to $errors
	errors=$("$@" 2>&1 1>&${stdout})
	ret=$?
	printf '%s\n' "$errors" >&2

?  Or something like that (for example I'm not sure if you need the \n).

In either case, stdout and stderr can be mixed.

> +	temp_file RUNTIME_arch_run <(echo "#!/bin/bash"; cat scripts/arch-run.bash "$TEST_DIR/run")

Please use a pipe instead of a <(...):

	(echo "#! /bin/bash"
	 cat scripts/arch-run.bash "$TEST_DIR/run") | tempfile RUNTIME_arch_run

by changing this in temp_file:

	local file="$2"
	gzip - < $file

to

	local file="${2--}"
	gzip -c "$file"

Paolo