Hi Drew,
On Thu, May 08, 2025 at 11:39:54AM +0200, Andrew Jones wrote:
> 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.
I'm all for the function wrappers, I was planning to reply to that comment
later.
As to this patch, is this what you're thinking:
function vmm_optname_nr_cpus()
{
if [ -z $TARGET ]; then
echo vmm_opts[qemu:nr_cpus]
else
echo vmm_opts[$TARGET:nr_cpus]
fi
}
But checking if $TARGET is defined makes this patch useless, and I would
rather drop it if that's the case.
Thanks,
Alex
>
> Thanks,
> drew
>
