On Wed, 7 Nov 2018 10:33:21 +0100
Andrew Jones <drjones@xxxxxxxxxx> wrote:
> On Tue, Nov 06, 2018 at 12:05:05PM -0500, Luiz Capitulino wrote:
> > On Tue, 6 Nov 2018 17:48:52 +0100
> > Andrew Jones <drjones@xxxxxxxxxx> wrote:
> >
> > > On Tue, Nov 06, 2018 at 11:31:33AM -0500, Luiz Capitulino wrote:
> > > >
> > > > According to README.md, users can set QEMU_ACCEL to specify
> > > > kvm or tcg. However, get_qemu_acceletor() uses ACCEL instead,
> > > > which is wrong since ACCEL is an "internal" variable. The end
> > > > result is that tests will still run if the user wants to use
> > > > kvm but kvm is unavailable. Fix get_qemu_acceletor() to use
> > > > QEMU_ACCEL.
> > >
> > > Hi Luiz,
> > >
> > > I agree "ACCEL" isn't a very good name for an environment
> > > variable, which is why I reserved the name QEMU_ACCEL in
> > > the readme. However, ACCEL doesn't have to *only* be an
> > > internal variable, i.e. 'ACCEL=tcg ./run_tests.sh' currently
> > > works, and some users may be used to it now.
> >
> > For some reason I thought ACCEL=tcg ./run_tests.sh wouldn't
> > work, but you're right it does.
> >
> > > Can you elaborate on the problem you're solving?
> >
> > Long story short, I just needed QEMU_ACCEL=kvm ./run_tests.sh
> > to fail/skip tests if /dev/kvm is not available (instead of
> > switching to tcg by default). Since ACCEL works, my problem
> > is solved.
> >
> > IMO we need to update the README.md file or make QEMU_ACCEL
> > work.
> >
>
> We need to clarify the readme, and in general better document what
> variables do and how to use them. QEMU_ACCEL isn't wrong, it's just
> not very useful. It's purpose is for the guest code to determine if
> it's kvm or tcg, not the runtime bash scripts like ACCEL. You can
> use QEMU_ACCEL like this
>
> $ cat x86/output-accel.c
> #include <libcflat.h>
> void main(void)
> {
> printf("%s\n", getenv("QEMU_ACCEL"));
> }
>
> $ echo 'QEMU_ACCEL=ANY_STRING' > kvm_unit_tests_env
>
> $ ACCEL=tcg KVM_UNIT_TESTS_ENV=kvm_unit_tests_env x86/run x86/output-accel.flat
> /usr/bin/qemu-system-x86_64 -nodefaults -device pc-testdev -device isa-debug-exit,iobase=0xf4,iosize=0x4 -vnc none -serial stdio -device pci-testdev -machine accel=tcg -kernel x86/output-accel.flat -initrd kvm_unit_tests_env
> enabling apic
> ANY_STRING
>
> So using the same name for both won't necessarily avoid confusion,
> as they're actually two different things.
Ohhh, that makes a lot of sense now. Thanks for taking the time to
explain it!
So, what happened in my case was: due to another bug, the kvm modules
weren't loaded on my machine so I didn't have /dev/kvm. kvm-unit-tests
then switched automatically to tcg, which caused some tests to fail. So,
I went to the readme to see how I could force /dev/kvm usage and fail
if it wasn't available. This led me to QEMU_ACCEL, not ACCEL.
And I have a second question/request. Even when using ACCEL, if /dev/kvm
is not available all tests will be skipped but I think it's useful to
completely fail run_tests.sh instead right at the beginning. Can I add
an option to run_tests.sh to do that?
> Updating the readme and the wiki have been on my TODO for a long
> time. I'll really try to bump the priority.
I'd volunteer, but I have to learn more about kvm-unit-tests. But I
think we probably want to separate documentation for those writing
unit-tests and for those who just want to run them.
