Re: [PATCH kvmtool] Skip a few messages by default: command line args; flat binary; earlyprintk.

[Will Deacon <will.deacon@arm.com>]

On Wed, Sep 30, 2015 at 05:11:15PM +0100, Andre Przywara wrote:
> On 29/09/15 17:59, Dimitri John Ledkov wrote:
> > The partial command line args & earlyprintk=serial are still enabled
> > in the debug mode. Warning that a flat binary kernel image is attemped
> > to be loaded is completely dropped. These are not that informative,
> > once one is past intial debugging, and only polute the console.
> > 
> > Signed-off-by: Dimitri John Ledkov <dimitri.j.ledkov@intel.com>
> > ---
> >  builtin-run.c | 10 ++++++----
> >  kvm.c         |  1 -
> >  x86/kvm.c     |  8 ++++++--
> >  3 files changed, 12 insertions(+), 7 deletions(-)
> > 
> > diff --git a/builtin-run.c b/builtin-run.c
> > index e0c8732..8edbf88 100644
> > --- a/builtin-run.c
> > +++ b/builtin-run.c
> > @@ -613,10 +613,12 @@ static struct kvm *kvm_cmd_run_init(int argc, const char **argv)
> >  
> >  	kvm->cfg.real_cmdline = real_cmdline;
> >  
> > -	printf("  # %s run -k %s -m %Lu -c %d --name %s\n", KVM_BINARY_NAME,
> > -		kvm->cfg.kernel_filename,
> > -		(unsigned long long)kvm->cfg.ram_size / 1024 / 1024,
> > -		kvm->cfg.nrcpus, kvm->cfg.guest_name);
> > +	if (do_debug_print) {
> > +		printf("  # %s run -k %s -m %Lu -c %d --name %s\n", KVM_BINARY_NAME,
> > +		       kvm->cfg.kernel_filename,
> > +		       (unsigned long long)kvm->cfg.ram_size / 1024 / 1024,
> > +		       kvm->cfg.nrcpus, kvm->cfg.guest_name);
> > +	}
> 
> I like the general idea. In fact I have this very patch (among others)
> in my tree too. I applied similar guarding to other messages as well
> (mostly those that only show up on ARM, but also the "ended normally"
> message). Like any good UNIX tool kvmtool should keep quiet if it has
> nothing worthwhile to say ;-)
> But looking at it more closely, I see that there is pr_debug() defined
> doing that "if (do_debug_print)" already. The only issue is that is
> prints source line information, which is not really useful here. But
> then again there does not seem to be any user of it?
> 
> So what about the following:
> - We avoid printing pr_info() messages in the default case. Looking at
> its current users in the tree this information is not really useful for
> normal users. We enable pr_info() output only if do_debug_print is
> enabled or introduce another command line flag (--verbose?) for that.
> - We check each user of pr_info() to see whether this information is
> actually "informational" or whether it should be converted to pr_warn.
> - We change the above line to use pr_info instead of printf.
> - We fix the EOL mayhem we have atm while at it.
> 
> If you don't mind I will give this a try later this week.

Sounds good to me.

Will