Re: [kvm-unit-tests PATCH 1/3] arch-run: Extend timeout when booting with UEFI

From: Nikos Nikoleris <nikos.nikoleris@arm.com>
To: Andrew Jones <andrew.jones@linux.dev>, kvm@vger.kernel.org
Cc: Paolo Bonzini <pbonzini@redhat.com>, Thomas Huth <thuth@redhat.com>
Subject: Re: [kvm-unit-tests PATCH 1/3] arch-run: Extend timeout when booting with UEFI
Date: Thu, 15 Jun 2023 15:46:52 +0100	[thread overview]
Message-ID: <9a1a4e84-6567-fde2-945c-9ceb40e42c9f@arm.com> (raw)
In-Reply-To: <20230607185905.32810-2-andrew.jones@linux.dev>

On 07/06/2023 19:59, Andrew Jones wrote:
> Booting UEFI can take a long time. Give the timeout some extra time
> to compensate for it.
> 
> Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
> ---
>   scripts/arch-run.bash | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> index 51e4b97b27d1..72ce718b1170 100644
> --- a/scripts/arch-run.bash
> +++ b/scripts/arch-run.bash
> @@ -94,7 +94,17 @@ run_qemu_status ()
>   
>   timeout_cmd ()
>   {
> +	local s
> +
>   	if [ "$TIMEOUT" ] && [ "$TIMEOUT" != "0" ]; then
> +		if [ "$CONFIG_EFI" = 'y' ]; then
> +			s=${TIMEOUT: -1}
> +			if [ "$s" = 's' ]; then
> +				TIMEOUT=${TIMEOUT:0:-1}
> +				((TIMEOUT += 10)) # Add 10 seconds for booting UEFI
> +				TIMEOUT="${TIMEOUT}s"
> +			fi
> +		fi
>   		echo "timeout -k 1s --foreground $TIMEOUT"
>   	fi
>   }

This looks fine to me but at the same time, I wonder if it's worth the 
complexity. In arm/unittests.cfg, timer is the only test where we 
specify a timeout. If we were to bump it from 10s to 20s it would solve 
the problem too but also the timeout would be extended for non EFI runs too.

In any case:

Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>

Thanks,

Nikos