* [PATCH v3 02/16] x86: Convert debug printfs to QEMU_DPRINTF
[not found] <1400367823-32610-1-git-send-email-marc.mari.barcelo@gmail.com>
@ 2014-05-17 23:03 ` Marc Marí
2014-05-19 15:35 ` [Qemu-devel] " Eric Blake
` (2 more replies)
2014-05-17 23:03 ` [PATCH v3 03/16] s390: " Marc Marí
1 sibling, 3 replies; 11+ messages in thread
From: Marc Marí @ 2014-05-17 23:03 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Crosthwaite, Andreas Färber, Stefan Hajnoczi,
Marc Marí, open list:X86, open list:X86
Modify debug macros to have the same format through the codebase and use regular
ifs instead of ifdef.
Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
---
hw/i386/kvm/pci-assign.c | 11 ++++++-----
hw/i386/multiboot.c | 7 +++++--
target-i386/kvm.c | 9 +++++----
xen-hvm.c | 9 +++++----
xen-mapcache.c | 9 +++++----
5 files changed, 26 insertions(+), 19 deletions(-)
diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index de33657..21330a9 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -51,14 +51,15 @@
//#define DEVICE_ASSIGNMENT_DEBUG
#ifdef DEVICE_ASSIGNMENT_DEBUG
-#define DEBUG(fmt, ...) \
- do { \
- fprintf(stderr, "%s: " fmt, __func__ , __VA_ARGS__); \
- } while (0)
+#define DEVICE_ASSIGNMENT_DEBUG_ENABLED 1
#else
-#define DEBUG(fmt, ...)
+#define DEVICE_ASSIGNMENT_DEBUG_ENABLED 0
#endif
+#define DEBUG(fmt, ...) \
+ QEMU_DPRINTF(DEVICE_ASSIGNMENT_DEBUG_ENABLED, \
+ "pci_assign", fmt, ## __VA_ARGS__)
+
typedef struct PCIRegion {
int type; /* Memory or port I/O */
int valid;
diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
index 985ca1e..cd04339 100644
--- a/hw/i386/multiboot.c
+++ b/hw/i386/multiboot.c
@@ -33,11 +33,14 @@
//#define DEBUG_MULTIBOOT
#ifdef DEBUG_MULTIBOOT
-#define mb_debug(a...) fprintf(stderr, ## a)
+#define DEBUG_MULTIBOOT_ENABLED 1
#else
-#define mb_debug(a...)
+#define DEBUG_MULTIBOOT_ENABLED 0
#endif
+#define mb_debug(...) \
+ QEMU_DPRINTF(DEBUG_MULTIBOOT_ENABLED, "i386 multiboot", __VA_ARGS__)
+
#define MULTIBOOT_STRUCT_ADDR 0x9000
#if MULTIBOOT_STRUCT_ADDR > 0xf0000
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 0d894ef..53098ad 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -39,13 +39,14 @@
//#define DEBUG_KVM
#ifdef DEBUG_KVM
-#define DPRINTF(fmt, ...) \
- do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
+#define DEBUG_KVM_ENABLED 1
#else
-#define DPRINTF(fmt, ...) \
- do { } while (0)
+#define DEBUG_KVM_ENABLED 0
#endif
+#define DPRINTF(fmt, ...) \
+ QEMU_DPRINTF(DEBUG_KVM_ENABLED, "i386 kvm", fmt, ## __VA_ARGS__)
+
#define MSR_KVM_WALL_CLOCK 0x11
#define MSR_KVM_SYSTEM_TIME 0x12
diff --git a/xen-hvm.c b/xen-hvm.c
index a64486c..a0d0bf1 100644
--- a/xen-hvm.c
+++ b/xen-hvm.c
@@ -29,13 +29,14 @@
//#define DEBUG_XEN_HVM
#ifdef DEBUG_XEN_HVM
-#define DPRINTF(fmt, ...) \
- do { fprintf(stderr, "xen: " fmt, ## __VA_ARGS__); } while (0)
+#define DEBUG_XEN_HVM_ENABLED 1
#else
-#define DPRINTF(fmt, ...) \
- do { } while (0)
+#define DEBUG_XEN_HVM_ENABLED 0
#endif
+#define DPRINTF(fmt, ...) \
+ QEMU_DPRINTF(DEBUG_XEN_HVM_ENABLED, "xen", fmt, ## __VA_ARGS__)
+
static MemoryRegion ram_memory, ram_640k, ram_lo, ram_hi;
static MemoryRegion *framebuffer;
static bool xen_in_migration;
diff --git a/xen-mapcache.c b/xen-mapcache.c
index eda914a..c6a72c5 100644
--- a/xen-mapcache.c
+++ b/xen-mapcache.c
@@ -26,13 +26,14 @@
//#define MAPCACHE_DEBUG
#ifdef MAPCACHE_DEBUG
-# define DPRINTF(fmt, ...) do { \
- fprintf(stderr, "xen_mapcache: " fmt, ## __VA_ARGS__); \
-} while (0)
+#define MAPCACHE_DEBUG_ENABLED 1
#else
-# define DPRINTF(fmt, ...) do { } while (0)
+#define MAPCACHE_DEBUG_ENABLED 0
#endif
+#define DPRINTF(fmt, ...) \
+ QEMU_DPRINTF(MAPCACHE_DEBUG_ENABLED, "xen_mapcache", fmt, ## __VA_ARGS__)
+
#if defined(__i386__)
# define MCACHE_BUCKET_SHIFT 16
# define MCACHE_MAX_SIZE (1UL<<31) /* 2GB Cap */
--
1.7.10.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 03/16] s390: Convert debug printfs to QEMU_DPRINTF
[not found] <1400367823-32610-1-git-send-email-marc.mari.barcelo@gmail.com>
2014-05-17 23:03 ` [PATCH v3 02/16] x86: Convert debug printfs to QEMU_DPRINTF Marc Marí
@ 2014-05-17 23:03 ` Marc Marí
2014-05-18 9:37 ` [Qemu-devel] " Peter Crosthwaite
1 sibling, 1 reply; 11+ messages in thread
From: Marc Marí @ 2014-05-17 23:03 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Crosthwaite, Andreas Färber, Stefan Hajnoczi,
Marc Marí, open list:Overall
Modify debug macros to have the same format through the codebase and use regular
ifs instead of ifdef.
Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
---
hw/s390x/s390-virtio-bus.c | 9 +++++----
hw/s390x/s390-virtio.c | 9 +++++----
target-s390x/helper.c | 23 +++++++++++++++--------
target-s390x/kvm.c | 9 +++++----
4 files changed, 30 insertions(+), 20 deletions(-)
diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index 9c71afa..2a1799e 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -38,13 +38,14 @@
/* #define DEBUG_S390 */
#ifdef DEBUG_S390
-#define DPRINTF(fmt, ...) \
- do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
+#define DEBUG_S390_ENABLED 1
#else
-#define DPRINTF(fmt, ...) \
- do { } while (0)
+#define DEBUG_S390_ENABLED 0
#endif
+#define DPRINTF(fmt, ...) \
+ QEMU_DPRINTF(DEBUG_S390_ENABLED, "s390 virtio bus", fmt, ## __VA_ARGS__)
+
#define VIRTIO_EXT_CODE 0x2603
static void virtio_s390_bus_new(VirtioBusState *bus, size_t bus_size,
diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c
index aef2003..b69afb4 100644
--- a/hw/s390x/s390-virtio.c
+++ b/hw/s390x/s390-virtio.c
@@ -42,13 +42,14 @@
//#define DEBUG_S390
#ifdef DEBUG_S390
-#define DPRINTF(fmt, ...) \
- do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
+#define DEBUG_S390_ENABLED 1
#else
-#define DPRINTF(fmt, ...) \
- do { } while (0)
+#define DEBUG_S390_ENABLED 0
#endif
+#define DPRINTF(fmt, ...) \
+ QEMU_DPRINTF(DEBUG_S390_ENABLED, "s390 virtio", fmt, ## __VA_ARGS__)
+
#define MAX_BLK_DEVS 10
#define ZIPL_FILENAME "s390-zipl.rom"
diff --git a/target-s390x/helper.c b/target-s390x/helper.c
index 7c76fc1..c2aa568 100644
--- a/target-s390x/helper.c
+++ b/target-s390x/helper.c
@@ -30,19 +30,26 @@
//#define DEBUG_S390_STDOUT
#ifdef DEBUG_S390
-#ifdef DEBUG_S390_STDOUT
-#define DPRINTF(fmt, ...) \
- do { fprintf(stderr, fmt, ## __VA_ARGS__); \
- qemu_log(fmt, ##__VA_ARGS__); } while (0)
+#define DEBUG_S390_ENABLED 1
#else
-#define DPRINTF(fmt, ...) \
- do { qemu_log(fmt, ## __VA_ARGS__); } while (0)
+#define DEBUG_S390_ENABLED 0
#endif
+
+#ifdef DEBUG_S390_STDOUT
+#define DEBUG_S390_STDOUT_ENABLED 1
#else
-#define DPRINTF(fmt, ...) \
- do { } while (0)
+#define DEBUG_S390_STDOUT_ENABLED 0
#endif
+#define DPRINTF(fmt, ...) \
+ do { \
+ if(DEBUG_S390_ENABLED) { \
+ qemu_log(fmt, ##__VA_ARGS__); \
+ QEMU_DPRINTF(DEBUG_S390_STDOUT_ENABLED, "s390x helper", \
+ fmt, ## __VA_ARGS__); \
+ } \
+ } while (0)
+
#ifdef DEBUG_S390_PTE
#define PTE_DPRINTF DPRINTF
#else
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 56179af..63c46c4 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -41,13 +41,14 @@
/* #define DEBUG_KVM */
#ifdef DEBUG_KVM
-#define DPRINTF(fmt, ...) \
- do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
+#define DEBUG_KVM_ENABLED 1
#else
-#define DPRINTF(fmt, ...) \
- do { } while (0)
+#define DEBUG_KVM_ENABLED 0
#endif
+#define DPRINTF(fmt, ...) \
+ QEMU_DPRINTF(DEBUG_KVM_ENABLED, "s390 kvm", fmt, ## __VA_ARGS__)
+
#define IPA0_DIAG 0x8300
#define IPA0_SIGP 0xae00
#define IPA0_B2 0xb200
--
1.7.10.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v3 03/16] s390: Convert debug printfs to QEMU_DPRINTF
2014-05-17 23:03 ` [PATCH v3 03/16] s390: " Marc Marí
@ 2014-05-18 9:37 ` Peter Crosthwaite
2014-05-19 11:50 ` Alexander Graf
2014-05-19 16:45 ` [Qemu-devel] " Marc Marí
0 siblings, 2 replies; 11+ messages in thread
From: Peter Crosthwaite @ 2014-05-18 9:37 UTC (permalink / raw)
To: Marc Marí, Richard Henderson, Alexander Graf
Cc: qemu-devel@nongnu.org Developers, Stefan Hajnoczi,
Andreas Färber, open list:Overall
On Sun, May 18, 2014 at 9:03 AM, Marc Marí <marc.mari.barcelo@gmail.com> wrote:
> Modify debug macros to have the same format through the codebase and use regular
> ifs instead of ifdef.
>
> Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
> ---
> hw/s390x/s390-virtio-bus.c | 9 +++++----
> hw/s390x/s390-virtio.c | 9 +++++----
> target-s390x/helper.c | 23 +++++++++++++++--------
> target-s390x/kvm.c | 9 +++++----
> 4 files changed, 30 insertions(+), 20 deletions(-)
>
> diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
> index 9c71afa..2a1799e 100644
> --- a/hw/s390x/s390-virtio-bus.c
> +++ b/hw/s390x/s390-virtio-bus.c
> @@ -38,13 +38,14 @@
> /* #define DEBUG_S390 */
>
> #ifdef DEBUG_S390
> -#define DPRINTF(fmt, ...) \
> - do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
> +#define DEBUG_S390_ENABLED 1
> #else
> -#define DPRINTF(fmt, ...) \
> - do { } while (0)
> +#define DEBUG_S390_ENABLED 0
> #endif
>
> +#define DPRINTF(fmt, ...) \
> + QEMU_DPRINTF(DEBUG_S390_ENABLED, "s390 virtio bus", fmt, ## __VA_ARGS__)
> +
> #define VIRTIO_EXT_CODE 0x2603
>
> static void virtio_s390_bus_new(VirtioBusState *bus, size_t bus_size,
> diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c
> index aef2003..b69afb4 100644
> --- a/hw/s390x/s390-virtio.c
> +++ b/hw/s390x/s390-virtio.c
> @@ -42,13 +42,14 @@
> //#define DEBUG_S390
>
> #ifdef DEBUG_S390
> -#define DPRINTF(fmt, ...) \
> - do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
> +#define DEBUG_S390_ENABLED 1
> #else
> -#define DPRINTF(fmt, ...) \
> - do { } while (0)
> +#define DEBUG_S390_ENABLED 0
> #endif
>
> +#define DPRINTF(fmt, ...) \
> + QEMU_DPRINTF(DEBUG_S390_ENABLED, "s390 virtio", fmt, ## __VA_ARGS__)
> +
> #define MAX_BLK_DEVS 10
> #define ZIPL_FILENAME "s390-zipl.rom"
>
> diff --git a/target-s390x/helper.c b/target-s390x/helper.c
> index 7c76fc1..c2aa568 100644
> --- a/target-s390x/helper.c
> +++ b/target-s390x/helper.c
> @@ -30,19 +30,26 @@
> //#define DEBUG_S390_STDOUT
>
> #ifdef DEBUG_S390
> -#ifdef DEBUG_S390_STDOUT
> -#define DPRINTF(fmt, ...) \
> - do { fprintf(stderr, fmt, ## __VA_ARGS__); \
> - qemu_log(fmt, ##__VA_ARGS__); } while (0)
> +#define DEBUG_S390_ENABLED 1
> #else
> -#define DPRINTF(fmt, ...) \
> - do { qemu_log(fmt, ## __VA_ARGS__); } while (0)
> +#define DEBUG_S390_ENABLED 0
> #endif
> +
> +#ifdef DEBUG_S390_STDOUT
This STDOUT vs _log() choice is a bit irregular, and I think you might
be better off abandoning it completely. Richard, Alex, do we really
need to optionally route printfery to log or stderr? (considering _log
is NOW stderr by default now, and -D option gives you some flexibility
there). Can we have just log and drop STDOUT mode?
Regards,
Peter
> +#define DEBUG_S390_STDOUT_ENABLED 1
> #else
> -#define DPRINTF(fmt, ...) \
> - do { } while (0)
> +#define DEBUG_S390_STDOUT_ENABLED 0
> #endif
>
> +#define DPRINTF(fmt, ...) \
> + do { \
> + if(DEBUG_S390_ENABLED) { \
> + qemu_log(fmt, ##__VA_ARGS__); \
> + QEMU_DPRINTF(DEBUG_S390_STDOUT_ENABLED, "s390x helper", \
> + fmt, ## __VA_ARGS__); \
> + } \
> + } while (0)
> +
> #ifdef DEBUG_S390_PTE
> #define PTE_DPRINTF DPRINTF
> #else
> diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
> index 56179af..63c46c4 100644
> --- a/target-s390x/kvm.c
> +++ b/target-s390x/kvm.c
> @@ -41,13 +41,14 @@
> /* #define DEBUG_KVM */
>
> #ifdef DEBUG_KVM
> -#define DPRINTF(fmt, ...) \
> - do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
> +#define DEBUG_KVM_ENABLED 1
> #else
> -#define DPRINTF(fmt, ...) \
> - do { } while (0)
> +#define DEBUG_KVM_ENABLED 0
> #endif
>
> +#define DPRINTF(fmt, ...) \
> + QEMU_DPRINTF(DEBUG_KVM_ENABLED, "s390 kvm", fmt, ## __VA_ARGS__)
> +
> #define IPA0_DIAG 0x8300
> #define IPA0_SIGP 0xae00
> #define IPA0_B2 0xb200
> --
> 1.7.10.4
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 03/16] s390: Convert debug printfs to QEMU_DPRINTF
2014-05-18 9:37 ` [Qemu-devel] " Peter Crosthwaite
@ 2014-05-19 11:50 ` Alexander Graf
2014-05-19 16:45 ` [Qemu-devel] " Marc Marí
1 sibling, 0 replies; 11+ messages in thread
From: Alexander Graf @ 2014-05-19 11:50 UTC (permalink / raw)
To: Peter Crosthwaite, Marc Marí, Richard Henderson
Cc: Stefan Hajnoczi, qemu-devel@nongnu.org Developers, Overall,
Andreas Färber
On 18.05.14 11:37, Peter Crosthwaite wrote:
> On Sun, May 18, 2014 at 9:03 AM, Marc Marí <marc.mari.barcelo@gmail.com> wrote:
>> Modify debug macros to have the same format through the codebase and use regular
>> ifs instead of ifdef.
>>
>> Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
>> ---
>> hw/s390x/s390-virtio-bus.c | 9 +++++----
>> hw/s390x/s390-virtio.c | 9 +++++----
>> target-s390x/helper.c | 23 +++++++++++++++--------
>> target-s390x/kvm.c | 9 +++++----
>> 4 files changed, 30 insertions(+), 20 deletions(-)
>>
>> diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
>> index 9c71afa..2a1799e 100644
>> --- a/hw/s390x/s390-virtio-bus.c
>> +++ b/hw/s390x/s390-virtio-bus.c
>> @@ -38,13 +38,14 @@
>> /* #define DEBUG_S390 */
>>
>> #ifdef DEBUG_S390
>> -#define DPRINTF(fmt, ...) \
>> - do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
>> +#define DEBUG_S390_ENABLED 1
>> #else
>> -#define DPRINTF(fmt, ...) \
>> - do { } while (0)
>> +#define DEBUG_S390_ENABLED 0
>> #endif
>>
>> +#define DPRINTF(fmt, ...) \
>> + QEMU_DPRINTF(DEBUG_S390_ENABLED, "s390 virtio bus", fmt, ## __VA_ARGS__)
>> +
>> #define VIRTIO_EXT_CODE 0x2603
>>
>> static void virtio_s390_bus_new(VirtioBusState *bus, size_t bus_size,
>> diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c
>> index aef2003..b69afb4 100644
>> --- a/hw/s390x/s390-virtio.c
>> +++ b/hw/s390x/s390-virtio.c
>> @@ -42,13 +42,14 @@
>> //#define DEBUG_S390
>>
>> #ifdef DEBUG_S390
>> -#define DPRINTF(fmt, ...) \
>> - do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
>> +#define DEBUG_S390_ENABLED 1
>> #else
>> -#define DPRINTF(fmt, ...) \
>> - do { } while (0)
>> +#define DEBUG_S390_ENABLED 0
>> #endif
>>
>> +#define DPRINTF(fmt, ...) \
>> + QEMU_DPRINTF(DEBUG_S390_ENABLED, "s390 virtio", fmt, ## __VA_ARGS__)
>> +
>> #define MAX_BLK_DEVS 10
>> #define ZIPL_FILENAME "s390-zipl.rom"
>>
>> diff --git a/target-s390x/helper.c b/target-s390x/helper.c
>> index 7c76fc1..c2aa568 100644
>> --- a/target-s390x/helper.c
>> +++ b/target-s390x/helper.c
>> @@ -30,19 +30,26 @@
>> //#define DEBUG_S390_STDOUT
>>
>> #ifdef DEBUG_S390
>> -#ifdef DEBUG_S390_STDOUT
>> -#define DPRINTF(fmt, ...) \
>> - do { fprintf(stderr, fmt, ## __VA_ARGS__); \
>> - qemu_log(fmt, ##__VA_ARGS__); } while (0)
>> +#define DEBUG_S390_ENABLED 1
>> #else
>> -#define DPRINTF(fmt, ...) \
>> - do { qemu_log(fmt, ## __VA_ARGS__); } while (0)
>> +#define DEBUG_S390_ENABLED 0
>> #endif
>> +
>> +#ifdef DEBUG_S390_STDOUT
> This STDOUT vs _log() choice is a bit irregular, and I think you might
> be better off abandoning it completely. Richard, Alex, do we really
> need to optionally route printfery to log or stderr? (considering _log
> is NOW stderr by default now, and -D option gives you some flexibility
> there). Can we have just log and drop STDOUT mode?
Works for me :).
Alex
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v3 02/16] x86: Convert debug printfs to QEMU_DPRINTF
2014-05-17 23:03 ` [PATCH v3 02/16] x86: Convert debug printfs to QEMU_DPRINTF Marc Marí
@ 2014-05-19 15:35 ` Eric Blake
2014-05-19 15:45 ` Eric Blake
2014-05-19 15:48 ` Eric Blake
2 siblings, 0 replies; 11+ messages in thread
From: Eric Blake @ 2014-05-19 15:35 UTC (permalink / raw)
To: Marc Marí, qemu-devel
Cc: Stefan Hajnoczi, Peter Crosthwaite, X86, Andreas Färber, X86
[-- Attachment #1.1: Type: text/plain, Size: 1482 bytes --]
On 05/17/2014 05:03 PM, Marc Marí wrote:
> Modify debug macros to have the same format through the codebase and use regular
> ifs instead of ifdef.
>
> Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
> ---
> hw/i386/kvm/pci-assign.c | 11 ++++++-----
> hw/i386/multiboot.c | 7 +++++--
> target-i386/kvm.c | 9 +++++----
> xen-hvm.c | 9 +++++----
> xen-mapcache.c | 9 +++++----
> 5 files changed, 26 insertions(+), 19 deletions(-)
>
> #ifdef DEVICE_ASSIGNMENT_DEBUG
> -#define DEBUG(fmt, ...) \
> - do { \
> - fprintf(stderr, "%s: " fmt, __func__ , __VA_ARGS__); \
> - } while (0)
> +#define DEVICE_ASSIGNMENT_DEBUG_ENABLED 1
> #else
> -#define DEBUG(fmt, ...)
> +#define DEVICE_ASSIGNMENT_DEBUG_ENABLED 0
> #endif
>
> +#define DEBUG(fmt, ...) \
> + QEMU_DPRINTF(DEVICE_ASSIGNMENT_DEBUG_ENABLED, \
> + "pci_assign", fmt, ## __VA_ARGS__)
> +
Style - you are replacing aligned \ continuation with one-space \
continuation. I don't know if we have a distinct preference, but it's
probably better to leave the style unchanged if checkpatch.pl doesn't
complain about either way.
As that's minor,
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v3 02/16] x86: Convert debug printfs to QEMU_DPRINTF
2014-05-17 23:03 ` [PATCH v3 02/16] x86: Convert debug printfs to QEMU_DPRINTF Marc Marí
2014-05-19 15:35 ` [Qemu-devel] " Eric Blake
@ 2014-05-19 15:45 ` Eric Blake
2014-05-19 15:48 ` Eric Blake
2 siblings, 0 replies; 11+ messages in thread
From: Eric Blake @ 2014-05-19 15:45 UTC (permalink / raw)
To: Marc Marí, qemu-devel
Cc: Stefan Hajnoczi, Peter Crosthwaite, X86, Andreas Färber, X86
[-- Attachment #1.1: Type: text/plain, Size: 1921 bytes --]
[resend, because my first attempt got:
----- The following addresses had permanent fatal errors -----
<qemu-devel@nongnu.org>
(reason: 550 Invalid address in message header)
I think it was complaining about:
CC: open list:X86 <xen-devel@lists.xensource.com>
CC: open list:X86 <kvm@vger.kernel.org>
where the RFCs say you should enclose : inside "", as in:
CC: "open list:X86" <...>
to be fully compliant.]
On 05/17/2014 05:03 PM, Marc Marí wrote:
> Modify debug macros to have the same format through the codebase and use regular
> ifs instead of ifdef.
>
> Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
> ---
> hw/i386/kvm/pci-assign.c | 11 ++++++-----
> hw/i386/multiboot.c | 7 +++++--
> target-i386/kvm.c | 9 +++++----
> xen-hvm.c | 9 +++++----
> xen-mapcache.c | 9 +++++----
> 5 files changed, 26 insertions(+), 19 deletions(-)
>
> #ifdef DEVICE_ASSIGNMENT_DEBUG
> -#define DEBUG(fmt, ...) \
> - do { \
> - fprintf(stderr, "%s: " fmt, __func__ , __VA_ARGS__); \
> - } while (0)
> +#define DEVICE_ASSIGNMENT_DEBUG_ENABLED 1
> #else
> -#define DEBUG(fmt, ...)
> +#define DEVICE_ASSIGNMENT_DEBUG_ENABLED 0
> #endif
>
> +#define DEBUG(fmt, ...) \
> + QEMU_DPRINTF(DEVICE_ASSIGNMENT_DEBUG_ENABLED, \
> + "pci_assign", fmt, ## __VA_ARGS__)
> +
Style - you are replacing aligned \ continuation with one-space \
continuation. I don't know if we have a distinct preference, but it's
probably better to leave the style unchanged if checkpatch.pl doesn't
complain about either way.
As that's minor,
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v3 02/16] x86: Convert debug printfs to QEMU_DPRINTF
2014-05-17 23:03 ` [PATCH v3 02/16] x86: Convert debug printfs to QEMU_DPRINTF Marc Marí
2014-05-19 15:35 ` [Qemu-devel] " Eric Blake
2014-05-19 15:45 ` Eric Blake
@ 2014-05-19 15:48 ` Eric Blake
2 siblings, 0 replies; 11+ messages in thread
From: Eric Blake @ 2014-05-19 15:48 UTC (permalink / raw)
To: Marc Marí, qemu-devel@nongnu.org
Cc: Peter Crosthwaite, xen-devel, kvm, Stefan Hajnoczi,
Andreas Färber
[-- Attachment #1: Type: text/plain, Size: 1906 bytes --]
[resend, because I got:
----- The following addresses had permanent fatal errors -----
<qemu-devel@nongnu.org>
(reason: 550 Invalid address in message header)
I think it was complaining about:
CC: open list:X86 <xen-devel@lists.xensource.com>
CC: open list:X86 <kvm@vger.kernel.org>
where the RFCs say you should enclose : inside "", as in:
CC: "open list:X86" <...>
to be fully compliant.]
On 05/17/2014 05:03 PM, Marc Marí wrote:
> Modify debug macros to have the same format through the codebase and use regular
> ifs instead of ifdef.
>
> Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
> ---
> hw/i386/kvm/pci-assign.c | 11 ++++++-----
> hw/i386/multiboot.c | 7 +++++--
> target-i386/kvm.c | 9 +++++----
> xen-hvm.c | 9 +++++----
> xen-mapcache.c | 9 +++++----
> 5 files changed, 26 insertions(+), 19 deletions(-)
>
> #ifdef DEVICE_ASSIGNMENT_DEBUG
> -#define DEBUG(fmt, ...) \
> - do { \
> - fprintf(stderr, "%s: " fmt, __func__ , __VA_ARGS__); \
> - } while (0)
> +#define DEVICE_ASSIGNMENT_DEBUG_ENABLED 1
> #else
> -#define DEBUG(fmt, ...)
> +#define DEVICE_ASSIGNMENT_DEBUG_ENABLED 0
> #endif
>
> +#define DEBUG(fmt, ...) \
> + QEMU_DPRINTF(DEVICE_ASSIGNMENT_DEBUG_ENABLED, \
> + "pci_assign", fmt, ## __VA_ARGS__)
> +
Style - you are replacing aligned \ continuation with one-space \
continuation. I don't know if we have a distinct preference, but it's
probably better to leave the style unchanged if checkpatch.pl doesn't
complain about either way.
As that's minor,
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v3 03/16] s390: Convert debug printfs to QEMU_DPRINTF
2014-05-18 9:37 ` [Qemu-devel] " Peter Crosthwaite
2014-05-19 11:50 ` Alexander Graf
@ 2014-05-19 16:45 ` Marc Marí
2014-05-19 16:50 ` Richard Henderson
1 sibling, 1 reply; 11+ messages in thread
From: Marc Marí @ 2014-05-19 16:45 UTC (permalink / raw)
To: Peter Crosthwaite
Cc: Richard Henderson, Alexander Graf,
qemu-devel@nongnu.org Developers, Stefan Hajnoczi,
Andreas Färber, open list:Overall
El Sun, 18 May 2014 19:37:12 +1000
Peter Crosthwaite <peter.crosthwaite@xilinx.com> escribió:
> On Sun, May 18, 2014 at 9:03 AM, Marc Marí
> <marc.mari.barcelo@gmail.com> wrote:
> > Modify debug macros to have the same format through the codebase
> > and use regular ifs instead of ifdef.
> >
> > Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
> > ---
> > hw/s390x/s390-virtio-bus.c | 9 +++++----
> > hw/s390x/s390-virtio.c | 9 +++++----
> > target-s390x/helper.c | 23 +++++++++++++++--------
> > target-s390x/kvm.c | 9 +++++----
> > 4 files changed, 30 insertions(+), 20 deletions(-)
> >
> > diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
> > index 9c71afa..2a1799e 100644
> > --- a/hw/s390x/s390-virtio-bus.c
> > +++ b/hw/s390x/s390-virtio-bus.c
> > @@ -38,13 +38,14 @@
> > /* #define DEBUG_S390 */
> >
> > #ifdef DEBUG_S390
> > -#define DPRINTF(fmt, ...) \
> > - do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
> > +#define DEBUG_S390_ENABLED 1
> > #else
> > -#define DPRINTF(fmt, ...) \
> > - do { } while (0)
> > +#define DEBUG_S390_ENABLED 0
> > #endif
> >
> > +#define DPRINTF(fmt, ...) \
> > + QEMU_DPRINTF(DEBUG_S390_ENABLED, "s390 virtio bus", fmt, ##
> > __VA_ARGS__) +
> > #define VIRTIO_EXT_CODE 0x2603
> >
> > static void virtio_s390_bus_new(VirtioBusState *bus, size_t
> > bus_size, diff --git a/hw/s390x/s390-virtio.c
> > b/hw/s390x/s390-virtio.c index aef2003..b69afb4 100644
> > --- a/hw/s390x/s390-virtio.c
> > +++ b/hw/s390x/s390-virtio.c
> > @@ -42,13 +42,14 @@
> > //#define DEBUG_S390
> >
> > #ifdef DEBUG_S390
> > -#define DPRINTF(fmt, ...) \
> > - do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
> > +#define DEBUG_S390_ENABLED 1
> > #else
> > -#define DPRINTF(fmt, ...) \
> > - do { } while (0)
> > +#define DEBUG_S390_ENABLED 0
> > #endif
> >
> > +#define DPRINTF(fmt, ...) \
> > + QEMU_DPRINTF(DEBUG_S390_ENABLED, "s390 virtio", fmt, ##
> > __VA_ARGS__) +
> > #define MAX_BLK_DEVS 10
> > #define ZIPL_FILENAME "s390-zipl.rom"
> >
> > diff --git a/target-s390x/helper.c b/target-s390x/helper.c
> > index 7c76fc1..c2aa568 100644
> > --- a/target-s390x/helper.c
> > +++ b/target-s390x/helper.c
> > @@ -30,19 +30,26 @@
> > //#define DEBUG_S390_STDOUT
> >
> > #ifdef DEBUG_S390
> > -#ifdef DEBUG_S390_STDOUT
> > -#define DPRINTF(fmt, ...) \
> > - do { fprintf(stderr, fmt, ## __VA_ARGS__); \
> > - qemu_log(fmt, ##__VA_ARGS__); } while (0)
> > +#define DEBUG_S390_ENABLED 1
> > #else
> > -#define DPRINTF(fmt, ...) \
> > - do { qemu_log(fmt, ## __VA_ARGS__); } while (0)
> > +#define DEBUG_S390_ENABLED 0
> > #endif
> > +
> > +#ifdef DEBUG_S390_STDOUT
>
> This STDOUT vs _log() choice is a bit irregular, and I think you might
> be better off abandoning it completely. Richard, Alex, do we really
> need to optionally route printfery to log or stderr? (considering _log
> is NOW stderr by default now, and -D option gives you some flexibility
> there). Can we have just log and drop STDOUT mode?
>
> Regards,
> Peter
QEMU_DPRINTF outputs to stderr, and, as you say qemu_log does it too.
Should QEMU_DPRINTF be removed and leave only qemu_log?
Marc
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 03/16] s390: Convert debug printfs to QEMU_DPRINTF
2014-05-19 16:45 ` [Qemu-devel] " Marc Marí
@ 2014-05-19 16:50 ` Richard Henderson
2014-05-19 20:33 ` [Qemu-devel] " Peter Crosthwaite
0 siblings, 1 reply; 11+ messages in thread
From: Richard Henderson @ 2014-05-19 16:50 UTC (permalink / raw)
To: Marc Marí, Peter Crosthwaite
Cc: Stefan Hajnoczi, Overall, Alexander Graf, Andreas Färber,
qemu-devel@nongnu.org Developers
On 05/19/2014 09:45 AM, Marc Marí wrote:
>> > This STDOUT vs _log() choice is a bit irregular, and I think you might
>> > be better off abandoning it completely. Richard, Alex, do we really
>> > need to optionally route printfery to log or stderr? (considering _log
>> > is NOW stderr by default now, and -D option gives you some flexibility
>> > there). Can we have just log and drop STDOUT mode?
>> >
>> > Regards,
>> > Peter
> QEMU_DPRINTF outputs to stderr, and, as you say qemu_log does it too.
> Should QEMU_DPRINTF be removed and leave only qemu_log?
Yes, I think that all debug output like this should go via qemu_log.
r~
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v3 03/16] s390: Convert debug printfs to QEMU_DPRINTF
2014-05-19 16:50 ` Richard Henderson
@ 2014-05-19 20:33 ` Peter Crosthwaite
2014-05-19 20:34 ` Alexander Graf
0 siblings, 1 reply; 11+ messages in thread
From: Peter Crosthwaite @ 2014-05-19 20:33 UTC (permalink / raw)
To: Richard Henderson
Cc: Marc Marí, Stefan Hajnoczi, Overall, Alexander Graf,
Andreas Färber, qemu-devel@nongnu.org Developers
On Tue, May 20, 2014 at 2:50 AM, Richard Henderson <rth@twiddle.net> wrote:
> On 05/19/2014 09:45 AM, Marc Marí wrote:
>>> > This STDOUT vs _log() choice is a bit irregular, and I think you might
>>> > be better off abandoning it completely. Richard, Alex, do we really
>>> > need to optionally route printfery to log or stderr? (considering _log
>>> > is NOW stderr by default now, and -D option gives you some flexibility
>>> > there). Can we have just log and drop STDOUT mode?
>>> >
>>> > Regards,
>>> > Peter
>> QEMU_DPRINTF outputs to stderr, and, as you say qemu_log does it too.
>> Should QEMU_DPRINTF be removed and leave only qemu_log?
Ahh I see now, you want to use the common factored-out code in P1
which is stderr specific. Perhaps that should use qemu_log. One option
would be to create a variant of it in common code that does uses
qemu_log instead of stderr and use that variant here. That way you can
preserve existing behaviour (and maintainer expectations), as much as
possible.
>
> Yes, I think that all debug output like this should go via qemu_log.
>
Thanks.
Regards,
Peter
>
> r~
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v3 03/16] s390: Convert debug printfs to QEMU_DPRINTF
2014-05-19 20:33 ` [Qemu-devel] " Peter Crosthwaite
@ 2014-05-19 20:34 ` Alexander Graf
0 siblings, 0 replies; 11+ messages in thread
From: Alexander Graf @ 2014-05-19 20:34 UTC (permalink / raw)
To: Peter Crosthwaite, Richard Henderson
Cc: Marc Marí, Stefan Hajnoczi, Overall, Andreas Färber,
qemu-devel@nongnu.org Developers
On 19.05.14 22:33, Peter Crosthwaite wrote:
> On Tue, May 20, 2014 at 2:50 AM, Richard Henderson <rth@twiddle.net> wrote:
>> On 05/19/2014 09:45 AM, Marc Marí wrote:
>>>>> This STDOUT vs _log() choice is a bit irregular, and I think you might
>>>>> be better off abandoning it completely. Richard, Alex, do we really
>>>>> need to optionally route printfery to log or stderr? (considering _log
>>>>> is NOW stderr by default now, and -D option gives you some flexibility
>>>>> there). Can we have just log and drop STDOUT mode?
>>>>>
>>>>> Regards,
>>>>> Peter
>>> QEMU_DPRINTF outputs to stderr, and, as you say qemu_log does it too.
>>> Should QEMU_DPRINTF be removed and leave only qemu_log?
> Ahh I see now, you want to use the common factored-out code in P1
> which is stderr specific. Perhaps that should use qemu_log. One option
> would be to create a variant of it in common code that does uses
> qemu_log instead of stderr and use that variant here. That way you can
> preserve existing behaviour (and maintainer expectations), as much as
> possible.
I think it's safe to assume that all TCG code wants to use qemu_log.
Alex
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2014-05-19 20:35 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1400367823-32610-1-git-send-email-marc.mari.barcelo@gmail.com>
2014-05-17 23:03 ` [PATCH v3 02/16] x86: Convert debug printfs to QEMU_DPRINTF Marc Marí
2014-05-19 15:35 ` [Qemu-devel] " Eric Blake
2014-05-19 15:45 ` Eric Blake
2014-05-19 15:48 ` Eric Blake
2014-05-17 23:03 ` [PATCH v3 03/16] s390: " Marc Marí
2014-05-18 9:37 ` [Qemu-devel] " Peter Crosthwaite
2014-05-19 11:50 ` Alexander Graf
2014-05-19 16:45 ` [Qemu-devel] " Marc Marí
2014-05-19 16:50 ` Richard Henderson
2014-05-19 20:33 ` [Qemu-devel] " Peter Crosthwaite
2014-05-19 20:34 ` Alexander Graf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).