| Message ID | 20170815163959.6632-2-f4bug@amsat.org |
|---|---|
| State | New |
| Headers | show |
On 08/15/2017 11:39 AM, Philippe Mathieu-Daudé wrote: > only warn once about it. > > - kernel without kvm: > > # make check-qtest-x86_64 > GTESTER check-qtest-x86_64 > Could not access KVM kernel module: No such device > qemu-system-x86_64: failed to initialize KVM: No such device > qemu-system-x86_64: Back to tcg accelerator How does this differ from what commit 2f6b38d1 was trying to do?
On 08/15/2017 03:13 PM, Eric Blake wrote: > On 08/15/2017 11:39 AM, Philippe Mathieu-Daudé wrote: >> only warn once about it. >> >> - kernel without kvm: >> >> # make check-qtest-x86_64 >> GTESTER check-qtest-x86_64 >> Could not access KVM kernel module: No such device >> qemu-system-x86_64: failed to initialize KVM: No such device >> qemu-system-x86_64: Back to tcg accelerator > > How does this differ from what commit 2f6b38d1 was trying to do? I'd say 2f6b38d1 is for common end-user usage while this is for QA/testers usage. If you run N qtests with the same machine arguments, having this warning displayed only once is enough and allow you to focus on the tests output.
On 15 August 2017 at 19:27, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > On 08/15/2017 03:13 PM, Eric Blake wrote: >> >> On 08/15/2017 11:39 AM, Philippe Mathieu-Daudé wrote: >>> >>> only warn once about it. >>> >>> - kernel without kvm: >>> >>> # make check-qtest-x86_64 >>> GTESTER check-qtest-x86_64 >>> Could not access KVM kernel module: No such device >>> qemu-system-x86_64: failed to initialize KVM: No such device >>> qemu-system-x86_64: Back to tcg accelerator >> >> >> How does this differ from what commit 2f6b38d1 was trying to do? > > > I'd say 2f6b38d1 is for common end-user usage while this is for QA/testers > usage. If you run N qtests with the same machine arguments, having this > warning displayed only once is enough and allow you to focus on the tests > output. I think I'd rather we just straightforwardly didn't complain at all in the 'make check' output. There's no benefit to this for all these tests, especially given that they pretty much don't care whether they run under KVM or TCG because they're not trying to test that. (If they *are* trying to test KVM specific functionality then they should either skip the test quietly if KVM isn't present or they should fail it; printing useless waffle to the logs isn't helpful IMHO.) thanks -- PMM
On 08/15/2017 01:27 PM, Philippe Mathieu-Daudé wrote: > On 08/15/2017 03:13 PM, Eric Blake wrote: >> On 08/15/2017 11:39 AM, Philippe Mathieu-Daudé wrote: >>> only warn once about it. >>> >>> - kernel without kvm: >>> >>> # make check-qtest-x86_64 >>> GTESTER check-qtest-x86_64 >>> Could not access KVM kernel module: No such device >>> qemu-system-x86_64: failed to initialize KVM: No such device >>> qemu-system-x86_64: Back to tcg accelerator >> >> How does this differ from what commit 2f6b38d1 was trying to do? > > I'd say 2f6b38d1 is for common end-user usage while this is for > QA/testers usage. If you run N qtests with the same machine arguments, > having this warning displayed only once is enough and allow you to focus > on the tests output. I guess my question is: Are we still getting a warning output even with 2f6b38d1 applied? If so, why, because the point of that commit was to avoid the warning.
On 08/15/2017 01:40 PM, Eric Blake wrote: > On 08/15/2017 01:27 PM, Philippe Mathieu-Daudé wrote: >> On 08/15/2017 03:13 PM, Eric Blake wrote: >>> On 08/15/2017 11:39 AM, Philippe Mathieu-Daudé wrote: >>>> only warn once about it. >>>> >>>> - kernel without kvm: >>>> >>>> # make check-qtest-x86_64 >>>> GTESTER check-qtest-x86_64 >>>> Could not access KVM kernel module: No such device >>>> qemu-system-x86_64: failed to initialize KVM: No such device >>>> qemu-system-x86_64: Back to tcg accelerator >>> >>> How does this differ from what commit 2f6b38d1 was trying to do? >> >> I'd say 2f6b38d1 is for common end-user usage while this is for >> QA/testers usage. If you run N qtests with the same machine arguments, >> having this warning displayed only once is enough and allow you to focus >> on the tests output. > > I guess my question is: Are we still getting a warning output even with > 2f6b38d1 applied? If so, why, because the point of that commit was to > avoid the warning. And answering my own question: yes, the builds are still verbose when done in a VM that lacks nested kvm. :(
On 08/15/2017 03:40 PM, Eric Blake wrote: > On 08/15/2017 01:27 PM, Philippe Mathieu-Daudé wrote: >> On 08/15/2017 03:13 PM, Eric Blake wrote: >>> On 08/15/2017 11:39 AM, Philippe Mathieu-Daudé wrote: >>>> only warn once about it. >>>> >>>> - kernel without kvm: >>>> >>>> # make check-qtest-x86_64 >>>> GTESTER check-qtest-x86_64 >>>> Could not access KVM kernel module: No such device >>>> qemu-system-x86_64: failed to initialize KVM: No such device >>>> qemu-system-x86_64: Back to tcg accelerator >>> >>> How does this differ from what commit 2f6b38d1 was trying to do? >> >> I'd say 2f6b38d1 is for common end-user usage while this is for >> QA/testers usage. If you run N qtests with the same machine arguments, >> having this warning displayed only once is enough and allow you to focus >> on the tests output. > > I guess my question is: Are we still getting a warning output even with > 2f6b38d1 applied? If so, why, because the point of that commit was to > avoid the warning. The function this patch adds is called by gtester to prepare the QEMU command line options, usually this is done once in main(), then each qtest is run with the same QEMU command line. Once started QEMU calls accel_init_machine() displaying warnings from 303d4e865b7. Having 3 lines of warnings for each test is way too verbose.
On 08/15/2017 03:39 PM, Peter Maydell wrote: > On 15 August 2017 at 19:27, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: >> On 08/15/2017 03:13 PM, Eric Blake wrote: >>> >>> On 08/15/2017 11:39 AM, Philippe Mathieu-Daudé wrote: >>>> >>>> only warn once about it. >>>> >>>> - kernel without kvm: >>>> >>>> # make check-qtest-x86_64 >>>> GTESTER check-qtest-x86_64 >>>> Could not access KVM kernel module: No such device >>>> qemu-system-x86_64: failed to initialize KVM: No such device >>>> qemu-system-x86_64: Back to tcg accelerator >>> >>> >>> How does this differ from what commit 2f6b38d1 was trying to do? >> >> >> I'd say 2f6b38d1 is for common end-user usage while this is for QA/testers >> usage. If you run N qtests with the same machine arguments, having this >> warning displayed only once is enough and allow you to focus on the tests >> output. > > I think I'd rather we just straightforwardly didn't complain at > all in the 'make check' output. There's no benefit to this > for all these tests, especially given that they pretty much > don't care whether they run under KVM or TCG because they're Oh you mean checking qtest_enabled() within configure_accelerator()? Ok, clever :) > not trying to test that. (If they *are* trying to test KVM > specific functionality then they should either skip the > test quietly if KVM isn't present or they should fail it; > printing useless waffle to the logs isn't helpful IMHO.) > > thanks > -- PMM >
diff --git a/tests/libqtest.h b/tests/libqtest.h index 38bc1e9953..24e03148eb 100644 --- a/tests/libqtest.h +++ b/tests/libqtest.h @@ -927,4 +927,12 @@ QDict *qmp_fd(int fd, const char *fmt, ...); */ void qtest_cb_for_every_machine(void (*cb)(const char *machine)); +/** + * qtest_accel: + * @accel: List of accelerators + * + * Filter accelerators accessible on the host. + */ +const char *qtest_accel(const char *accel); + #endif diff --git a/tests/libqtest.c b/tests/libqtest.c index b9a1f180e1..d2dfca35a3 100644 --- a/tests/libqtest.c +++ b/tests/libqtest.c @@ -987,3 +987,21 @@ void qtest_cb_for_every_machine(void (*cb)(const char *machine)) qtest_end(); QDECREF(response); } + +const char *qtest_accel(const char *accel) +{ + static bool kvm_accessible = true; + + if (strlen(accel) <= 4 || strncmp(accel, "kvm:", 4)) { + return accel; /* no match */ + } + + if (!kvm_accessible || !access("/dev/kvm", W_OK)) { + accel += 4; /* skip "kvm:" */ + if (kvm_accessible) { + kvm_accessible = false; /* warn once */ + g_printerr("kvm not accessible, using %s\n", accel); + } + } + return accel; +}
only warn once about it. - kernel without kvm: # make check-qtest-x86_64 GTESTER check-qtest-x86_64 Could not access KVM kernel module: No such device qemu-system-x86_64: failed to initialize KVM: No such device qemu-system-x86_64: Back to tcg accelerator - tests ran as user: $ make check-qtest-x86_64 GTESTER check-qtest-x86_64 Could not access KVM kernel module: Permission denied qemu-system-x86_64: failed to initialize KVM: Permission denied qemu-system-x86_64: Back to tcg accelerator Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- tests/libqtest.h | 8 ++++++++ tests/libqtest.c | 18 ++++++++++++++++++ 2 files changed, 26 insertions(+)