The current 'DEBUG_KVM' macro is defined in many files, and turning on
the debug switch requires code modification, which is very inconvenient,
so this series add an option to configure to support the definition of
the 'DEBUG_KVM' macro.
In addition, patches 3 and 4 also make printf always compile in debug output
which will prevent bitrot of the format strings by referring to the
commit(08564ecd: s390x/kvm: make printf always compile in debug output).

alexchen (4):
  configure: Add a --enable-debug-kvm option to configure
  kvm: replace DEBUG_KVM to CONFIG_DEBUG_KVM
  kvm: make printf always compile in debug output
  i386/kvm: make printf always compile in debug output

 accel/kvm/kvm-all.c | 11 ++++-------
 configure           | 10 ++++++++++
 target/i386/kvm.c   | 11 ++++-------
 target/mips/kvm.c   |  6 ++++--
 target/s390x/kvm.c  |  6 +++---
 5 files changed, 25 insertions(+), 19 deletions(-)

-- 
2.19.1

On 28/10/20 08:11, AlexChen wrote:
> The current 'DEBUG_KVM' macro is defined in many files, and turning on
> the debug switch requires code modification, which is very inconvenient,
> so this series add an option to configure to support the definition of
> the 'DEBUG_KVM' macro.
> In addition, patches 3 and 4 also make printf always compile in debug output
> which will prevent bitrot of the format strings by referring to the
> commit(08564ecd: s390x/kvm: make printf always compile in debug output).

Mostly we should use tracepoints, but the usefulness of these printf
statements is often limited (except for s390 that maybe could make them
unconditional error_reports).  I would leave this as is, maintainers can
decide which tracepoints they like to have.

Paolo

On Wed, 28 Oct 2020 08:44:59 +0100
Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 28/10/20 08:11, AlexChen wrote:
> > The current 'DEBUG_KVM' macro is defined in many files, and turning on
> > the debug switch requires code modification, which is very inconvenient,
> > so this series add an option to configure to support the definition of
> > the 'DEBUG_KVM' macro.
> > In addition, patches 3 and 4 also make printf always compile in debug output
> > which will prevent bitrot of the format strings by referring to the
> > commit(08564ecd: s390x/kvm: make printf always compile in debug output).  
> 
> Mostly we should use tracepoints, but the usefulness of these printf
> statements is often limited (except for s390 that maybe could make them
> unconditional error_reports).  I would leave this as is, maintainers can
> decide which tracepoints they like to have.

Looking at the s390 statements, they look more like something to put
into trace events (the "unimplemented instruction" cases are handled
gracefully, but the information might be interesting when developing.)

On 2020/10/28 15:44, Paolo Bonzini wrote:
> On 28/10/20 08:11, AlexChen wrote:
>> The current 'DEBUG_KVM' macro is defined in many files, and turning on
>> the debug switch requires code modification, which is very inconvenient,
>> so this series add an option to configure to support the definition of
>> the 'DEBUG_KVM' macro.
>> In addition, patches 3 and 4 also make printf always compile in debug output
>> which will prevent bitrot of the format strings by referring to the
>> commit(08564ecd: s390x/kvm: make printf always compile in debug output).
> 
> Mostly we should use tracepoints, but the usefulness of these printf
> statements is often limited (except for s390 that maybe could make them
> unconditional error_reports).  I would leave this as is, maintainers can
> decide which tracepoints they like to have.
> 
Thanks for your review, I agree with you to leave 'DEBUG_KVM' as is.
In addition, patches 3 and 4 resolved the potential risk of bitrot of the format strings,
could you help review these two patches?

Thanks,
Alex