All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] kvm: Add a --enable-debug-kvm option to configure
       [not found] <5F97FD61.4060804@huawei.com>
@ 2020-10-28  7:11   ` AlexChen
       [not found] ` <5F991331.4020604@huawei.com>
  1 sibling, 0 replies; 16+ messages in thread
From: AlexChen @ 2020-10-28  7:11 UTC (permalink / raw)
  To: pbonzini, chenhc, pasic, borntraeger, mtosatti, cohuck
  Cc: kvm, qemu-devel, qemu-s390x, zhengchuan, zhang.zhanghailiang

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 0/4] kvm: Add a --enable-debug-kvm option to configure
@ 2020-10-28  7:11   ` AlexChen
  0 siblings, 0 replies; 16+ messages in thread
From: AlexChen @ 2020-10-28  7:11 UTC (permalink / raw)
  To: pbonzini, chenhc, pasic, borntraeger, mtosatti, cohuck
  Cc: zhengchuan, qemu-s390x, qemu-devel, kvm, zhang.zhanghailiang

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


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/4] configure: Add a --enable-debug-kvm option to configure
       [not found] ` <5F991331.4020604@huawei.com>
@ 2020-10-28  7:11     ` AlexChen
  2020-10-28  7:11     ` AlexChen
       [not found]   ` <5F9914EE.8050209@huawei.com>
  2 siblings, 0 replies; 16+ messages in thread
From: AlexChen @ 2020-10-28  7:11 UTC (permalink / raw)
  To: pbonzini, chenhc, pasic, borntraeger, mtosatti, cohuck
  Cc: kvm, qemu-devel, qemu-s390x, zhengchuan, zhang.zhanghailiang

This patch allows CONFIG_DEBUG_KVM to be defined when passing
an option to the configure script.

Signed-off-by: AlexChen <alex.chen@huawei.com>
---
 configure | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/configure b/configure
index e6754c1e87..2cdef5be4c 100755
--- a/configure
+++ b/configure
@@ -338,6 +338,7 @@ pvrdma=""
 gprof="no"
 debug_tcg="no"
 debug="no"
+debug_kvm="no"
 sanitizers="no"
 tsan="no"
 fortify_source=""
@@ -1022,11 +1023,16 @@ for opt do
   ;;
   --disable-debug-tcg) debug_tcg="no"
   ;;
+  --enable-debug-kvm) debug_kvm="yes"
+  ;;
+  --disable-debug-kvm) debug_kvm="no"
+  ;;
   --enable-debug)
       # Enable debugging options that aren't excessively noisy
       debug_tcg="yes"
       debug_mutex="yes"
       debug="yes"
+      debug_kvm="yes"
       strip_opt="no"
       fortify_source="no"
   ;;
@@ -1735,6 +1741,7 @@ disabled with --disable-FEATURE, default is enabled if available:
   module-upgrades try to load modules from alternate paths for upgrades
   debug-tcg       TCG debugging (default is disabled)
   debug-info      debugging information
+  debug-kvm       KVM debugging support (default is disabled)
   sparse          sparse checker
   safe-stack      SafeStack Stack Smash Protection. Depends on
                   clang/llvm >= 3.7 and requires coroutine backend ucontext.
@@ -5929,6 +5936,9 @@ fi
 if test "$debug_tcg" = "yes" ; then
   echo "CONFIG_DEBUG_TCG=y" >> $config_host_mak
 fi
+if test "$debug_kvm" = "yes" ; then
+  echo "CONFIG_DEBUG_KVM=y" >> $config_host_mak
+fi
 if test "$strip_opt" = "yes" ; then
   echo "STRIP=${strip}" >> $config_host_mak
 fi
-- 
2.19.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 1/4] configure: Add a --enable-debug-kvm option to configure
@ 2020-10-28  7:11     ` AlexChen
  0 siblings, 0 replies; 16+ messages in thread
From: AlexChen @ 2020-10-28  7:11 UTC (permalink / raw)
  To: pbonzini, chenhc, pasic, borntraeger, mtosatti, cohuck
  Cc: zhengchuan, qemu-s390x, qemu-devel, kvm, zhang.zhanghailiang

This patch allows CONFIG_DEBUG_KVM to be defined when passing
an option to the configure script.

Signed-off-by: AlexChen <alex.chen@huawei.com>
---
 configure | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/configure b/configure
index e6754c1e87..2cdef5be4c 100755
--- a/configure
+++ b/configure
@@ -338,6 +338,7 @@ pvrdma=""
 gprof="no"
 debug_tcg="no"
 debug="no"
+debug_kvm="no"
 sanitizers="no"
 tsan="no"
 fortify_source=""
@@ -1022,11 +1023,16 @@ for opt do
   ;;
   --disable-debug-tcg) debug_tcg="no"
   ;;
+  --enable-debug-kvm) debug_kvm="yes"
+  ;;
+  --disable-debug-kvm) debug_kvm="no"
+  ;;
   --enable-debug)
       # Enable debugging options that aren't excessively noisy
       debug_tcg="yes"
       debug_mutex="yes"
       debug="yes"
+      debug_kvm="yes"
       strip_opt="no"
       fortify_source="no"
   ;;
@@ -1735,6 +1741,7 @@ disabled with --disable-FEATURE, default is enabled if available:
   module-upgrades try to load modules from alternate paths for upgrades
   debug-tcg       TCG debugging (default is disabled)
   debug-info      debugging information
+  debug-kvm       KVM debugging support (default is disabled)
   sparse          sparse checker
   safe-stack      SafeStack Stack Smash Protection. Depends on
                   clang/llvm >= 3.7 and requires coroutine backend ucontext.
@@ -5929,6 +5936,9 @@ fi
 if test "$debug_tcg" = "yes" ; then
   echo "CONFIG_DEBUG_TCG=y" >> $config_host_mak
 fi
+if test "$debug_kvm" = "yes" ; then
+  echo "CONFIG_DEBUG_KVM=y" >> $config_host_mak
+fi
 if test "$strip_opt" = "yes" ; then
   echo "STRIP=${strip}" >> $config_host_mak
 fi
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/4] kvm: Replace DEBUG_KVM with CONFIG_DEBUG_KVM
       [not found] ` <5F991331.4020604@huawei.com>
@ 2020-10-28  7:11     ` AlexChen
  2020-10-28  7:11     ` AlexChen
       [not found]   ` <5F9914EE.8050209@huawei.com>
  2 siblings, 0 replies; 16+ messages in thread
From: AlexChen @ 2020-10-28  7:11 UTC (permalink / raw)
  To: pbonzini, chenhc, pasic, borntraeger, mtosatti, cohuck
  Cc: kvm, qemu-devel, qemu-s390x, zhengchuan, zhang.zhanghailiang

Now we can control the definition of DPRINTF by CONFIG_DEBUG_KVM,
so let's replace DEBUG_KVM with CONFIG_DEBUG_KVM.

Signed-off-by: AlexChen <alex.chen@huawei.com>
---
 accel/kvm/kvm-all.c | 3 +--
 target/i386/kvm.c   | 4 +---
 target/mips/kvm.c   | 6 ++++--
 target/s390x/kvm.c  | 6 +++---
 4 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 9ef5daf4c5..fc6d99a731 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -60,9 +60,8 @@
  */
 #define PAGE_SIZE qemu_real_host_page_size

-//#define DEBUG_KVM

-#ifdef DEBUG_KVM
+#ifdef CONFIG_DEBUG_KVM
 #define DPRINTF(fmt, ...) \
     do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
 #else
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index cf46259534..3e9344aed5 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -50,9 +50,7 @@
 #include "exec/memattrs.h"
 #include "trace.h"

-//#define DEBUG_KVM
-
-#ifdef DEBUG_KVM
+#ifdef CONFIG_DEBUG_KVM
 #define DPRINTF(fmt, ...) \
     do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
 #else
diff --git a/target/mips/kvm.c b/target/mips/kvm.c
index 72637a1e02..a0b979e6d2 100644
--- a/target/mips/kvm.c
+++ b/target/mips/kvm.c
@@ -28,10 +28,12 @@
 #include "exec/memattrs.h"
 #include "hw/boards.h"

-#define DEBUG_KVM 0
+#ifndef CONFIG_DEBUG_KVM
+#define CONFIG_DEBUG_KVM 0
+#endif

 #define DPRINTF(fmt, ...) \
-    do { if (DEBUG_KVM) { fprintf(stderr, fmt, ## __VA_ARGS__); } } while (0)
+    do { if (CONFIG_DEBUG_KVM) { fprintf(stderr, fmt, ## __VA_ARGS__); } } while (0)

 static int kvm_mips_fpu_cap;
 static int kvm_mips_msa_cap;
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index f13eff688c..8bc9e1e468 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -52,12 +52,12 @@
 #include "hw/s390x/s390-virtio-hcall.h"
 #include "hw/s390x/pv.h"

-#ifndef DEBUG_KVM
-#define DEBUG_KVM  0
+#ifndef CONFIG_DEBUG_KVM
+#define CONFIG_DEBUG_KVM  0
 #endif

 #define DPRINTF(fmt, ...) do {                \
-    if (DEBUG_KVM) {                          \
+    if (CONFIG_DEBUG_KVM) {                          \
         fprintf(stderr, fmt, ## __VA_ARGS__); \
     }                                         \
 } while (0)
-- 
2.19.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/4] kvm: Replace DEBUG_KVM with CONFIG_DEBUG_KVM
@ 2020-10-28  7:11     ` AlexChen
  0 siblings, 0 replies; 16+ messages in thread
From: AlexChen @ 2020-10-28  7:11 UTC (permalink / raw)
  To: pbonzini, chenhc, pasic, borntraeger, mtosatti, cohuck
  Cc: zhengchuan, qemu-s390x, qemu-devel, kvm, zhang.zhanghailiang

Now we can control the definition of DPRINTF by CONFIG_DEBUG_KVM,
so let's replace DEBUG_KVM with CONFIG_DEBUG_KVM.

Signed-off-by: AlexChen <alex.chen@huawei.com>
---
 accel/kvm/kvm-all.c | 3 +--
 target/i386/kvm.c   | 4 +---
 target/mips/kvm.c   | 6 ++++--
 target/s390x/kvm.c  | 6 +++---
 4 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 9ef5daf4c5..fc6d99a731 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -60,9 +60,8 @@
  */
 #define PAGE_SIZE qemu_real_host_page_size

-//#define DEBUG_KVM

-#ifdef DEBUG_KVM
+#ifdef CONFIG_DEBUG_KVM
 #define DPRINTF(fmt, ...) \
     do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
 #else
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index cf46259534..3e9344aed5 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -50,9 +50,7 @@
 #include "exec/memattrs.h"
 #include "trace.h"

-//#define DEBUG_KVM
-
-#ifdef DEBUG_KVM
+#ifdef CONFIG_DEBUG_KVM
 #define DPRINTF(fmt, ...) \
     do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
 #else
diff --git a/target/mips/kvm.c b/target/mips/kvm.c
index 72637a1e02..a0b979e6d2 100644
--- a/target/mips/kvm.c
+++ b/target/mips/kvm.c
@@ -28,10 +28,12 @@
 #include "exec/memattrs.h"
 #include "hw/boards.h"

-#define DEBUG_KVM 0
+#ifndef CONFIG_DEBUG_KVM
+#define CONFIG_DEBUG_KVM 0
+#endif

 #define DPRINTF(fmt, ...) \
-    do { if (DEBUG_KVM) { fprintf(stderr, fmt, ## __VA_ARGS__); } } while (0)
+    do { if (CONFIG_DEBUG_KVM) { fprintf(stderr, fmt, ## __VA_ARGS__); } } while (0)

 static int kvm_mips_fpu_cap;
 static int kvm_mips_msa_cap;
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index f13eff688c..8bc9e1e468 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -52,12 +52,12 @@
 #include "hw/s390x/s390-virtio-hcall.h"
 #include "hw/s390x/pv.h"

-#ifndef DEBUG_KVM
-#define DEBUG_KVM  0
+#ifndef CONFIG_DEBUG_KVM
+#define CONFIG_DEBUG_KVM  0
 #endif

 #define DPRINTF(fmt, ...) do {                \
-    if (DEBUG_KVM) {                          \
+    if (CONFIG_DEBUG_KVM) {                          \
         fprintf(stderr, fmt, ## __VA_ARGS__); \
     }                                         \
 } while (0)
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/4] kvm: make printf always compile in debug output
       [not found]   ` <5F9914EE.8050209@huawei.com>
@ 2020-10-28  7:11       ` AlexChen
       [not found]     ` <5F991641.4050606@huawei.com>
  1 sibling, 0 replies; 16+ messages in thread
From: AlexChen @ 2020-10-28  7:11 UTC (permalink / raw)
  To: pbonzini, chenhc, pasic, borntraeger, mtosatti, cohuck
  Cc: kvm, qemu-devel, qemu-s390x, zhengchuan, zhang.zhanghailiang

Wrapped printf calls inside debug macros (DPRINTF) in `if` statement.
This will ensure that printf function will always compile even if debug
output is turned off and, in turn, will prevent bitrot of the format
strings.

Signed-off-by: AlexChen <alex.chen@huawei.com>
---
 accel/kvm/kvm-all.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index fc6d99a731..854b352346 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -60,14 +60,12 @@
  */
 #define PAGE_SIZE qemu_real_host_page_size

+#ifndef CONFIG_DEBUG_KVM
+#define CONFIG_DEBUG_KVM 0
+#endif

-#ifdef CONFIG_DEBUG_KVM
-#define DPRINTF(fmt, ...) \
-    do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
-#else
 #define DPRINTF(fmt, ...) \
-    do { } while (0)
-#endif
+    do { if (CONFIG_DEBUG_KVM) { fprintf(stderr, fmt, ## __VA_ARGS__); } } while (0)

 #define KVM_MSI_HASHTAB_SIZE    256

-- 
2.19.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/4] kvm: make printf always compile in debug output
@ 2020-10-28  7:11       ` AlexChen
  0 siblings, 0 replies; 16+ messages in thread
From: AlexChen @ 2020-10-28  7:11 UTC (permalink / raw)
  To: pbonzini, chenhc, pasic, borntraeger, mtosatti, cohuck
  Cc: zhengchuan, qemu-s390x, qemu-devel, kvm, zhang.zhanghailiang

Wrapped printf calls inside debug macros (DPRINTF) in `if` statement.
This will ensure that printf function will always compile even if debug
output is turned off and, in turn, will prevent bitrot of the format
strings.

Signed-off-by: AlexChen <alex.chen@huawei.com>
---
 accel/kvm/kvm-all.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index fc6d99a731..854b352346 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -60,14 +60,12 @@
  */
 #define PAGE_SIZE qemu_real_host_page_size

+#ifndef CONFIG_DEBUG_KVM
+#define CONFIG_DEBUG_KVM 0
+#endif

-#ifdef CONFIG_DEBUG_KVM
-#define DPRINTF(fmt, ...) \
-    do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
-#else
 #define DPRINTF(fmt, ...) \
-    do { } while (0)
-#endif
+    do { if (CONFIG_DEBUG_KVM) { fprintf(stderr, fmt, ## __VA_ARGS__); } } while (0)

 #define KVM_MSI_HASHTAB_SIZE    256

-- 
2.19.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 4/4] i386/kvm: make printf always compile in debug output
       [not found]     ` <5F991641.4050606@huawei.com>
@ 2020-10-28  7:11         ` AlexChen
  0 siblings, 0 replies; 16+ messages in thread
From: AlexChen @ 2020-10-28  7:11 UTC (permalink / raw)
  To: pbonzini, chenhc, pasic, borntraeger, mtosatti, cohuck
  Cc: kvm, qemu-devel, qemu-s390x, zhengchuan, zhang.zhanghailiang

Wrapped printf calls inside debug macros (DPRINTF) in `if` statement.
This will ensure that printf function will always compile even if debug
output is turned off and, in turn, will prevent bitrot of the format
strings.

Signed-off-by: AlexChen <alex.chen@huawei.com>
---
 target/i386/kvm.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 3e9344aed5..64492cb851 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -50,14 +50,13 @@
 #include "exec/memattrs.h"
 #include "trace.h"

-#ifdef CONFIG_DEBUG_KVM
-#define DPRINTF(fmt, ...) \
-    do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
-#else
-#define DPRINTF(fmt, ...) \
-    do { } while (0)
+#ifndef CONFIG_DEBUG_KVM
+#define CONFIG_DEBUG_KVM  0
 #endif

+#define DPRINTF(fmt, ...) \
+    do { if (CONFIG_DEBUG_KVM) { fprintf(stderr, fmt, ## __VA_ARGS__); } } while (0)
+
 /* From arch/x86/kvm/lapic.h */
 #define KVM_APIC_BUS_CYCLE_NS       1
 #define KVM_APIC_BUS_FREQUENCY      (1000000000ULL / KVM_APIC_BUS_CYCLE_NS)
-- 
2.19.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 4/4] i386/kvm: make printf always compile in debug output
@ 2020-10-28  7:11         ` AlexChen
  0 siblings, 0 replies; 16+ messages in thread
From: AlexChen @ 2020-10-28  7:11 UTC (permalink / raw)
  To: pbonzini, chenhc, pasic, borntraeger, mtosatti, cohuck
  Cc: zhengchuan, qemu-s390x, qemu-devel, kvm, zhang.zhanghailiang

Wrapped printf calls inside debug macros (DPRINTF) in `if` statement.
This will ensure that printf function will always compile even if debug
output is turned off and, in turn, will prevent bitrot of the format
strings.

Signed-off-by: AlexChen <alex.chen@huawei.com>
---
 target/i386/kvm.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 3e9344aed5..64492cb851 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -50,14 +50,13 @@
 #include "exec/memattrs.h"
 #include "trace.h"

-#ifdef CONFIG_DEBUG_KVM
-#define DPRINTF(fmt, ...) \
-    do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
-#else
-#define DPRINTF(fmt, ...) \
-    do { } while (0)
+#ifndef CONFIG_DEBUG_KVM
+#define CONFIG_DEBUG_KVM  0
 #endif

+#define DPRINTF(fmt, ...) \
+    do { if (CONFIG_DEBUG_KVM) { fprintf(stderr, fmt, ## __VA_ARGS__); } } while (0)
+
 /* From arch/x86/kvm/lapic.h */
 #define KVM_APIC_BUS_CYCLE_NS       1
 #define KVM_APIC_BUS_FREQUENCY      (1000000000ULL / KVM_APIC_BUS_CYCLE_NS)
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/4] kvm: Add a --enable-debug-kvm option to configure
  2020-10-28  7:11   ` AlexChen
@ 2020-10-28  7:44     ` Paolo Bonzini
  -1 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2020-10-28  7:44 UTC (permalink / raw)
  To: AlexChen, chenhc, pasic, borntraeger, mtosatti, cohuck
  Cc: kvm, qemu-devel, qemu-s390x, zhengchuan, zhang.zhanghailiang

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


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/4] kvm: Add a --enable-debug-kvm option to configure
@ 2020-10-28  7:44     ` Paolo Bonzini
  0 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2020-10-28  7:44 UTC (permalink / raw)
  To: AlexChen, chenhc, pasic, borntraeger, mtosatti, cohuck
  Cc: zhengchuan, qemu-s390x, qemu-devel, kvm, zhang.zhanghailiang

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



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/4] kvm: Add a --enable-debug-kvm option to configure
  2020-10-28  7:44     ` Paolo Bonzini
@ 2020-10-28  8:20       ` Cornelia Huck
  -1 siblings, 0 replies; 16+ messages in thread
From: Cornelia Huck @ 2020-10-28  8:20 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: AlexChen, chenhc, pasic, borntraeger, mtosatti, kvm, qemu-devel,
	qemu-s390x, zhengchuan, zhang.zhanghailiang

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.)


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/4] kvm: Add a --enable-debug-kvm option to configure
@ 2020-10-28  8:20       ` Cornelia Huck
  0 siblings, 0 replies; 16+ messages in thread
From: Cornelia Huck @ 2020-10-28  8:20 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: pasic, zhang.zhanghailiang, kvm, mtosatti, qemu-devel, AlexChen,
	borntraeger, qemu-s390x, chenhc, zhengchuan

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.)



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/4] kvm: Add a --enable-debug-kvm option to configure
  2020-10-28  7:44     ` Paolo Bonzini
@ 2020-10-29  6:13       ` AlexChen
  -1 siblings, 0 replies; 16+ messages in thread
From: AlexChen @ 2020-10-29  6:13 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: chenhc, pasic, borntraeger, mtosatti, cohuck, zhengchuan,
	qemu-s390x, qemu-devel, kvm, zhang.zhanghailiang

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/4] kvm: Add a --enable-debug-kvm option to configure
@ 2020-10-29  6:13       ` AlexChen
  0 siblings, 0 replies; 16+ messages in thread
From: AlexChen @ 2020-10-29  6:13 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: zhengchuan, mtosatti, zhang.zhanghailiang, kvm, cohuck,
	qemu-devel, pasic, borntraeger, qemu-s390x, chenhc

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


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2020-10-29  8:42 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <5F97FD61.4060804@huawei.com>
2020-10-28  7:11 ` [PATCH 0/4] kvm: Add a --enable-debug-kvm option to configure AlexChen
2020-10-28  7:11   ` AlexChen
2020-10-28  7:44   ` Paolo Bonzini
2020-10-28  7:44     ` Paolo Bonzini
2020-10-28  8:20     ` Cornelia Huck
2020-10-28  8:20       ` Cornelia Huck
2020-10-29  6:13     ` AlexChen
2020-10-29  6:13       ` AlexChen
     [not found] ` <5F991331.4020604@huawei.com>
2020-10-28  7:11   ` [PATCH 1/4] configure: " AlexChen
2020-10-28  7:11     ` AlexChen
2020-10-28  7:11   ` [PATCH 2/4] kvm: Replace DEBUG_KVM with CONFIG_DEBUG_KVM AlexChen
2020-10-28  7:11     ` AlexChen
     [not found]   ` <5F9914EE.8050209@huawei.com>
2020-10-28  7:11     ` [PATCH 3/4] kvm: make printf always compile in debug output AlexChen
2020-10-28  7:11       ` AlexChen
     [not found]     ` <5F991641.4050606@huawei.com>
2020-10-28  7:11       ` [PATCH 4/4] i386/kvm: " AlexChen
2020-10-28  7:11         ` AlexChen

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.