On Thu, May 08, 2025 at 09:52:38AM +0100, Alexandru Elisei wrote:
> Hi Drew,
> 
> On Wed, May 07, 2025 at 06:02:31PM +0200, Andrew Jones wrote:
> > On Wed, May 07, 2025 at 04:12:43PM +0100, Alexandru Elisei wrote:
> > > Only arm and arm64 are allowed to set --target to kvmtool; the rest of the
> > > architectures can only set --target to 'qemu', which is also the default.
> > > 
> > > Needed to make the changes necessary to add support for kvmtool to the 
> > > test
> > > runner.
> > > 
> > > kvmtool also supports running the riscv tests, so it's not outside of the
> > > realm of the possibily for the riscv tests to get support for kvmtool.
> > > 
> > > Signed-off-by: Alexandru Elisei <alexandru.eli...@arm.com>
> > > ---
> > >  configure | 36 ++++++++++++++++++++++++------------
> > >  1 file changed, 24 insertions(+), 12 deletions(-)
> > >
> > 
> > Reviewed-by: Andrew Jones <andrew.jo...@linux.dev>
> 
> Thank you for the review!
> 
> Just to be clear, you are ok with this happening because of the patch:
> 
> $ git pull
> $ make clean && make
> $ ./run_tests.sh
> scripts/runtime.bash: line 24: scripts/arch-run.bash: line 444: [: =: unary 
> operator expected
> timeout -k 1s --foreground 90s /usr/bin/qemu-system-x86_64 --no-reboot 
> -nodefaults -device pc-testdev -device isa-debug-exit,iobase=0xf4,iosize=0x4 
> -vnc none -serial stdio -device pci-testdev -machine accel=kvm -kernel 
> _NO_FILE_4Uhere_ 2 #  /tmp/tmp.bME9I2BZRG
> qemu-system-x86_64: 2: Could not open '2': No such file or directory
> scripts/arch-run.bash: line 19: 1: command not found: No such file or 
> directory
> FAIL apic-split
> scripts/runtime.bash: line 24: scripts/arch-run.bash: line 444: [: =: unary 
> operator expected
> timeout -k 1s --foreground 90s /usr/bin/qemu-system-x86_64 --no-reboot 
> -nodefaults -device pc-testdev -device isa-debug-exit,iobase=0xf4,iosize=0x4 
> -vnc none -serial stdio -device pci-testdev -machine accel=kvm -kernel 
> _NO_FILE_4Uhere_ 1 #  /tmp/tmp.11und6qZbL
> qemu-system-x86_64: 1: Could not open '1': No such file or directory
> scripts/arch-run.bash: line 19: 1: command not found: No such file or 
> directory
> FAIL ioapic-split
> [..]
> 
> That's because TARGET is missing from config.mak. If you're ok with the
> error, I'll make it clear in the commit message why this is happening.
>

It's not ideal, but I think it's pretty common to run configure before
make after an update to the git repo, so it's not horrible. However,
as you pointed out in your cover letter, this can be mitigated if we
use function wrappers for the associative array accesses, allowing
$TARGET to be checked before it's used. I'd prefer the function wrappers
anyway for readability reasons, so let's do that.

Thanks,
drew

Reply via email to