| Message ID | 20210923144505.60776-9-alexandru.elisei@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Run kvm-unit-tests with --kernel | expand |
On Thu, Sep 23, 2021 at 03:45:03PM +0100, Alexandru Elisei wrote: > Commit e66942073035 ("kvm tools: Guest kernel compatability") added the > functionality that enables devices to print a warning message if the device > hasn't been initialized by the time the VM is destroyed. The purpose of > these messages is to let the user know if the kernel hasn't been built with > the correct Kconfig options to take advantage of the said devices (all > using virtio). > > Since then, kvmtool has evolved and now supports loading different payloads > (like firmware images), and having those warnings even when it is entirely > intentional for the payload not to touch the devices can be confusing for > the user and makes the output unnecessarily verbose in those cases. > > Add the --nocompat option to disable the warnings; the warnings are still > enabled by default. > > Reported-by: Christoffer Dall <christoffer.dall@arm.com> > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> > --- > builtin-run.c | 5 ++++- > guest_compat.c | 1 + > include/kvm/kvm-config.h | 1 + > 3 files changed, 6 insertions(+), 1 deletion(-) Sorry, bikeshed moment here, but why don't we just have a '--quiet' option that shuts everything up unless it's fatal? Will
Hi Will, On Tue, Oct 12, 2021 at 09:34:53AM +0100, Will Deacon wrote: > On Thu, Sep 23, 2021 at 03:45:03PM +0100, Alexandru Elisei wrote: > > Commit e66942073035 ("kvm tools: Guest kernel compatability") added the > > functionality that enables devices to print a warning message if the device > > hasn't been initialized by the time the VM is destroyed. The purpose of > > these messages is to let the user know if the kernel hasn't been built with > > the correct Kconfig options to take advantage of the said devices (all > > using virtio). > > > > Since then, kvmtool has evolved and now supports loading different payloads > > (like firmware images), and having those warnings even when it is entirely > > intentional for the payload not to touch the devices can be confusing for > > the user and makes the output unnecessarily verbose in those cases. > > > > Add the --nocompat option to disable the warnings; the warnings are still > > enabled by default. > > > > Reported-by: Christoffer Dall <christoffer.dall@arm.com> > > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> > > --- > > builtin-run.c | 5 ++++- > > guest_compat.c | 1 + > > include/kvm/kvm-config.h | 1 + > > 3 files changed, 6 insertions(+), 1 deletion(-) > > Sorry, bikeshed moment here, but why don't we just have a '--quiet' option > that shuts everything up unless it's fatal? I can't figure out what is the criteria for deciding what is silenced by --quiet and what is still shown. I chose --nocompat because it is clear what is supposed to disable. One possibility would be to hide pr_info() and the compat warnings. But that still doesn't feel right to me - why hide *only* the compat warnings and leave the other warnings unchanged? I could see having a --nocompat that hides the compat warningis *and* a quiet option that hides pr_info() output (grepping for pr_info reveals a number of places where it is used). What do you think? Do you have something else in mind? Thanks, Alex > > Will
diff --git a/builtin-run.c b/builtin-run.c index 9a1a0c1fa6fb..a736b63ce7e5 100644 --- a/builtin-run.c +++ b/builtin-run.c @@ -111,6 +111,8 @@ void kvm_run_set_wrapper_sandbox(void) OPT_BOOLEAN('\0', "nodefaults", &(cfg)->nodefaults, "Disable" \ " implicit configuration that cannot be" \ " disabled otherwise"), \ + OPT_BOOLEAN('\0', "nocompat", &(cfg)->nocompat, "Disable" \ + " compat warnings"), \ OPT_CALLBACK('\0', "9p", NULL, "dir_to_share,tag_name", \ "Enable virtio 9p to share files between host and" \ " guest", virtio_9p_rootdir_parser, kvm), \ @@ -709,7 +711,8 @@ static int kvm_cmd_run_work(struct kvm *kvm) static void kvm_cmd_run_exit(struct kvm *kvm, int guest_ret) { - compat__print_all_messages(); + if (!kvm->cfg.nocompat) + compat__print_all_messages(); init_list__exit(kvm); diff --git a/guest_compat.c b/guest_compat.c index fd4704b20b16..a413c12ccd2e 100644 --- a/guest_compat.c +++ b/guest_compat.c @@ -88,6 +88,7 @@ int compat__print_all_messages(void) printf("\n # KVM compatibility warning.\n\t%s\n\t%s\n", msg->title, msg->desc); + printf("\tTo stop seeing this warning, use the --nocompat option.\n"); list_del(&msg->list); compat__free(msg); diff --git a/include/kvm/kvm-config.h b/include/kvm/kvm-config.h index 6a5720c4c7d4..329aa46e6eda 100644 --- a/include/kvm/kvm-config.h +++ b/include/kvm/kvm-config.h @@ -28,6 +28,7 @@ struct kvm_config { u64 vsock_cid; bool virtio_rng; bool nodefaults; + bool nocompat; int active_console; int debug_iodelay; int nrcpus;
Commit e66942073035 ("kvm tools: Guest kernel compatability") added the functionality that enables devices to print a warning message if the device hasn't been initialized by the time the VM is destroyed. The purpose of these messages is to let the user know if the kernel hasn't been built with the correct Kconfig options to take advantage of the said devices (all using virtio). Since then, kvmtool has evolved and now supports loading different payloads (like firmware images), and having those warnings even when it is entirely intentional for the payload not to touch the devices can be confusing for the user and makes the output unnecessarily verbose in those cases. Add the --nocompat option to disable the warnings; the warnings are still enabled by default. Reported-by: Christoffer Dall <christoffer.dall@arm.com> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> --- builtin-run.c | 5 ++++- guest_compat.c | 1 + include/kvm/kvm-config.h | 1 + 3 files changed, 6 insertions(+), 1 deletion(-)