Re: [kvm-unit-tests PATCH v2 03/18] scripts: Refuse to run the tests if not configured for qemu

[Alexandru Elisei <alexandru.elisei@arm.com>]

Hi Drew,

On Tue, Jan 21, 2025 at 05:17:22PM +0100, Andrew Jones wrote:
> On Tue, Jan 21, 2025 at 03:54:17PM +0000, Alexandru Elisei wrote:
> > Hi Drew,
> > 
> > On Tue, Jan 21, 2025 at 03:48:55PM +0100, Andrew Jones wrote:
> > > On Mon, Jan 20, 2025 at 04:43:01PM +0000, Alexandru Elisei wrote:
> > > > Arm and arm64 support running the tests under kvmtool. Unsurprisingly,
> > > > kvmtool and qemu have a different command line syntax for configuring and
> > > > running a virtual machine.
> > > > 
> > > > On top of that, when kvm-unit-tests has been configured to run under
> > > > kvmtool (via ./configure --target=kvmtool), the early UART address changes,
> > > > and if then the tests are run with qemu, this warning is displayed:
> > > > 
> > > > WARNING: early print support may not work. Found uart at 0x9000000, but early base is 0x1000000.
> > > > 
> > > > At the moment, the only way to run a test under kvmtool is manually, as no
> > > > script has any knowledge of how to invoke kvmtool. Also, unless one looks
> > > > at the logs, it's not obvious that the test runner is using qemu to run the
> > > > tests, and not kvmtool.
> > > > 
> > > > To avoid any confusion for unsuspecting users, refuse to run a test via the
> > > > testing scripts when kvm-unit-tests has been configured for kvmtool.
> > > > 
> > > > There are four different ways to run a test using the test infrastructure:
> > > > with run_tests.sh, by invoking arm/run or arm/efi/run with the correct
> > > > parameters (only the arm directory is mentioned here because the tests can
> > > > be configured for kvmtool only on arm and arm64), and by creating
> > > > standalone tests. Add a check in each of these locations for the supported
> > > > virtual machine manager.
> > > > 
> > > > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> > > > ---
> > > >  arm/efi/run             | 8 ++++++++
> > > >  arm/run                 | 9 +++++++++
> > > >  run_tests.sh            | 8 ++++++++
> > > >  scripts/mkstandalone.sh | 8 ++++++++
> > > >  4 files changed, 33 insertions(+)
> > > > 
> > > > diff --git a/arm/efi/run b/arm/efi/run
> > > > index 8f41fc02df31..916f4c4deef6 100755
> > > > --- a/arm/efi/run
> > > > +++ b/arm/efi/run
> > > > @@ -12,6 +12,14 @@ fi
> > > >  source config.mak
> > > >  source scripts/arch-run.bash
> > > >  
> > > > +case "$TARGET" in
> > > > +qemu)
> > > > +    ;;
> > > > +*)
> > > > +    echo "$0 does not support '$TARGET'"
> > > > +    exit 2
> > > > +esac
> > > > +
> > > >  if [ -f /usr/share/qemu-efi-aarch64/QEMU_EFI.fd ]; then
> > > >  	DEFAULT_UEFI=/usr/share/qemu-efi-aarch64/QEMU_EFI.fd
> > > >  elif [ -f /usr/share/edk2/aarch64/QEMU_EFI.silent.fd ]; then
> > > > diff --git a/arm/run b/arm/run
> > > > index efdd44ce86a7..6db32cf09c88 100755
> > > > --- a/arm/run
> > > > +++ b/arm/run
> > > > @@ -8,6 +8,15 @@ if [ -z "$KUT_STANDALONE" ]; then
> > > >  	source config.mak
> > > >  	source scripts/arch-run.bash
> > > >  fi
> > > > +
> > > > +case "$TARGET" in
> > > > +qemu)
> > > > +    ;;
> > > > +*)
> > > > +   echo "'$TARGET' not supported"
> > > > +   exit 3
> > > 
> > > I think we want exit code 2 here.
> > 
> > Exit code 2 is already in use in arm/run. Now that I'm looking more closely
> > at it, exit code 2 is already in use in run_tests.sh, same for
> > mkstandalone.sh and arm/efi/run.
> > 
> > How about using 3 everywhere as the exit code?
> >
> 
> In kvm-unit-tests, exit code 2 is what we use for "most likely a run
> script failed" (see the comment above run_qemu() in
> scripts/arch-run.bash). We don't try to create a new error code for each
> type of error, but we do have the error message as well. So if there's a
> higher level runner, which runs this runner, it only needs to learn that
> 2 is likely a script failure and that an error message will hopefully
> point the way to the problem.
> 

I see, I missed the comment. Will change so it returns 2 everywhere.

Thanks,
Alex