All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexandru Elisei <alexandru.elisei@arm.com>
To: Will Deacon <will@kernel.org>
Cc: julien.thierry.kdev@gmail.com, kvm@vger.kernel.org,
	christoffer.dall@arm.com, vivek.gautam@arm.com
Subject: Re: [PATCH kvmtool 08/10] Add --nocompat option to disable compat warnings
Date: Tue, 12 Oct 2021 15:24:00 +0100	[thread overview]
Message-ID: <YWWagPVuugVaTD59@monolith.localdoman> (raw)
In-Reply-To: <20211012083452.GC5156@willie-the-truck>

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

  reply	other threads:[~2021-10-12 14:22 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-23 14:44 [PATCH kvmtool 00/10] Run kvm-unit-tests with --kernel Alexandru Elisei
2021-09-23 14:44 ` [PATCH kvmtool 01/10] builtin-run: Treat specifying both --kernel and --firmware as an error Alexandru Elisei
2021-09-23 14:44 ` [PATCH kvmtool 02/10] builtin-run: Warn when ignoring initrd because --firmware was specified Alexandru Elisei
2021-09-23 14:44 ` [PATCH kvmtool 03/10] builtin-run: Do not attempt to find vmlinux if --firmware Alexandru Elisei
2021-09-23 14:44 ` [PATCH kvmtool 04/10] builtin-run: Abstract argument validation into a separate function Alexandru Elisei
2021-09-23 14:45 ` [PATCH kvmtool 05/10] Use kvm->nr_disks instead of kvm->cfg.image_count Alexandru Elisei
2021-09-23 14:45 ` [PATCH kvmtool 06/10] builtin-run: Move kernel command line generation to a separate function Alexandru Elisei
2021-09-23 14:45 ` [PATCH kvmtool 07/10] Add --nodefaults command line argument Alexandru Elisei
2021-09-23 14:45 ` [PATCH kvmtool 08/10] Add --nocompat option to disable compat warnings Alexandru Elisei
2021-10-12  8:34   ` Will Deacon
2021-10-12 14:24     ` Alexandru Elisei [this message]
2021-09-23 14:45 ` [PATCH kvmtool 09/10] arm64: Use the default offset when the kernel image magic is not found Alexandru Elisei
2021-09-23 14:45 ` [PATCH kvmtool 10/10] arm64: Be more permissive when parsing the kernel header Alexandru Elisei
2021-10-12  8:46 ` [PATCH kvmtool 00/10] Run kvm-unit-tests with --kernel Will Deacon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YWWagPVuugVaTD59@monolith.localdoman \
    --to=alexandru.elisei@arm.com \
    --cc=christoffer.dall@arm.com \
    --cc=julien.thierry.kdev@gmail.com \
    --cc=kvm@vger.kernel.org \
    --cc=vivek.gautam@arm.com \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.