Patchwork [RFC,v3,03/21] virtproxy: add debug functions for virtproxy core

login
register
mail settings
Submitter Michael Roth
Date Nov. 16, 2010, 1:15 a.m.
Message ID <1289870175-14880-4-git-send-email-mdroth@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/71327/
State New
Headers show

Comments

Michael Roth - Nov. 16, 2010, 1:15 a.m.
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 virtproxy.c |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)
Jes Sorensen - Nov. 18, 2010, 11:09 a.m.
On 11/16/10 02:15, Michael Roth wrote:
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
>  virtproxy.c |   17 +++++++++++++++++
>  1 files changed, 17 insertions(+), 0 deletions(-)
> 
> diff --git a/virtproxy.c b/virtproxy.c
> index 8f18d83..3686c77 100644
> --- a/virtproxy.c
> +++ b/virtproxy.c
> @@ -13,6 +13,23 @@
>  
>  #include "virtproxy.h"
>  
> +#define DEBUG_VP
> +
> +#ifdef DEBUG_VP
> +#define TRACE(msg, ...) do { \
> +    fprintf(stderr, "%s:%s():L%d: " msg "\n", \
> +            __FILE__, __FUNCTION__, __LINE__, ## __VA_ARGS__); \
> +} while(0)
> +#else
> +#define TRACE(msg, ...) \
> +    do { } while (0)
> +#endif
> +
> +#define LOG(msg, ...) do { \
> +    fprintf(stderr, "%s:%s(): " msg "\n", \
> +            __FILE__, __FUNCTION__, ## __VA_ARGS__); \
> +} while(0)
> +

I wonder if it wouldn't make sense to do this in a more generic way and
stick it in a header file. This type of debug code seems to show up
repeatedly all over the place.

Cheers,
Jes
Stefan Hajnoczi - Nov. 18, 2010, 11:43 a.m.
On Thu, Nov 18, 2010 at 11:09 AM, Jes Sorensen <Jes.Sorensen@redhat.com> wrote:
> On 11/16/10 02:15, Michael Roth wrote:
>> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
>> ---
>>  virtproxy.c |   17 +++++++++++++++++
>>  1 files changed, 17 insertions(+), 0 deletions(-)
>>
>> diff --git a/virtproxy.c b/virtproxy.c
>> index 8f18d83..3686c77 100644
>> --- a/virtproxy.c
>> +++ b/virtproxy.c
>> @@ -13,6 +13,23 @@
>>
>>  #include "virtproxy.h"
>>
>> +#define DEBUG_VP
>> +
>> +#ifdef DEBUG_VP
>> +#define TRACE(msg, ...) do { \
>> +    fprintf(stderr, "%s:%s():L%d: " msg "\n", \
>> +            __FILE__, __FUNCTION__, __LINE__, ## __VA_ARGS__); \
>> +} while(0)
>> +#else
>> +#define TRACE(msg, ...) \
>> +    do { } while (0)
>> +#endif
>> +
>> +#define LOG(msg, ...) do { \
>> +    fprintf(stderr, "%s:%s(): " msg "\n", \
>> +            __FILE__, __FUNCTION__, ## __VA_ARGS__); \
>> +} while(0)
>> +
>
> I wonder if it wouldn't make sense to do this in a more generic way and
> stick it in a header file. This type of debug code seems to show up
> repeatedly all over the place.

I wanted to suggest actually using QEMU tracing but given that this
code compiles stand-alone, it may be more trouble to integrate than
it's worth.

For qemu and qemu-tools code we should be using tracing because you
can build it into production without high overhead.  How often do
these #defines get used, not often is my guess because no one wants to
tweak the source and rebuild, especially not in production.

Stefan
Michael Roth - Nov. 18, 2010, 5:17 p.m.
On 11/18/2010 05:43 AM, Stefan Hajnoczi wrote:
> On Thu, Nov 18, 2010 at 11:09 AM, Jes Sorensen<Jes.Sorensen@redhat.com>  wrote:
>> On 11/16/10 02:15, Michael Roth wrote:
>>> Signed-off-by: Michael Roth<mdroth@linux.vnet.ibm.com>
>>> ---
>>>   virtproxy.c |   17 +++++++++++++++++
>>>   1 files changed, 17 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/virtproxy.c b/virtproxy.c
>>> index 8f18d83..3686c77 100644
>>> --- a/virtproxy.c
>>> +++ b/virtproxy.c
>>> @@ -13,6 +13,23 @@
>>>
>>>   #include "virtproxy.h"
>>>
>>> +#define DEBUG_VP
>>> +
>>> +#ifdef DEBUG_VP
>>> +#define TRACE(msg, ...) do { \
>>> +    fprintf(stderr, "%s:%s():L%d: " msg "\n", \
>>> +            __FILE__, __FUNCTION__, __LINE__, ## __VA_ARGS__); \
>>> +} while(0)
>>> +#else
>>> +#define TRACE(msg, ...) \
>>> +    do { } while (0)
>>> +#endif
>>> +
>>> +#define LOG(msg, ...) do { \
>>> +    fprintf(stderr, "%s:%s(): " msg "\n", \
>>> +            __FILE__, __FUNCTION__, ## __VA_ARGS__); \
>>> +} while(0)
>>> +
>>
>> I wonder if it wouldn't make sense to do this in a more generic way and
>> stick it in a header file. This type of debug code seems to show up
>> repeatedly all over the place.
>
> I wanted to suggest actually using QEMU tracing but given that this
> code compiles stand-alone, it may be more trouble to integrate than
> it's worth.
>
> For qemu and qemu-tools code we should be using tracing because you
> can build it into production without high overhead.  How often do
> these #defines get used, not often is my guess because no one wants to
> tweak the source and rebuild, especially not in production.
>

I'd planned on replacing the TRACE()'s with trace calls eventually. 
Maybe I missed it in the documentation, but do we have a way to enable 
them for qemu-tools or does we need to provide our own hooks and do it 
programmatically?

> Stefan
Stefan Hajnoczi - Nov. 19, 2010, 9:21 a.m.
On Thu, Nov 18, 2010 at 5:17 PM, Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> On 11/18/2010 05:43 AM, Stefan Hajnoczi wrote:
>>
>> On Thu, Nov 18, 2010 at 11:09 AM, Jes Sorensen<Jes.Sorensen@redhat.com>
>>  wrote:
>>>
>>> On 11/16/10 02:15, Michael Roth wrote:
>>>>
>>>> Signed-off-by: Michael Roth<mdroth@linux.vnet.ibm.com>
>>>> ---
>>>>  virtproxy.c |   17 +++++++++++++++++
>>>>  1 files changed, 17 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/virtproxy.c b/virtproxy.c
>>>> index 8f18d83..3686c77 100644
>>>> --- a/virtproxy.c
>>>> +++ b/virtproxy.c
>>>> @@ -13,6 +13,23 @@
>>>>
>>>>  #include "virtproxy.h"
>>>>
>>>> +#define DEBUG_VP
>>>> +
>>>> +#ifdef DEBUG_VP
>>>> +#define TRACE(msg, ...) do { \
>>>> +    fprintf(stderr, "%s:%s():L%d: " msg "\n", \
>>>> +            __FILE__, __FUNCTION__, __LINE__, ## __VA_ARGS__); \
>>>> +} while(0)
>>>> +#else
>>>> +#define TRACE(msg, ...) \
>>>> +    do { } while (0)
>>>> +#endif
>>>> +
>>>> +#define LOG(msg, ...) do { \
>>>> +    fprintf(stderr, "%s:%s(): " msg "\n", \
>>>> +            __FILE__, __FUNCTION__, ## __VA_ARGS__); \
>>>> +} while(0)
>>>> +
>>>
>>> I wonder if it wouldn't make sense to do this in a more generic way and
>>> stick it in a header file. This type of debug code seems to show up
>>> repeatedly all over the place.
>>
>> I wanted to suggest actually using QEMU tracing but given that this
>> code compiles stand-alone, it may be more trouble to integrate than
>> it's worth.
>>
>> For qemu and qemu-tools code we should be using tracing because you
>> can build it into production without high overhead.  How often do
>> these #defines get used, not often is my guess because no one wants to
>> tweak the source and rebuild, especially not in production.
>>
>
> I'd planned on replacing the TRACE()'s with trace calls eventually. Maybe I
> missed it in the documentation, but do we have a way to enable them for
> qemu-tools or does we need to provide our own hooks and do it
> programmatically?

qemu-tools builds with QEMU tracing.  I wasn't sure whether you're
building in the qemu-tools style or whether qemu-vp is a completely
separate program.

So if qemu-vp is built with all the common qemu-tools make
infrastructure, then you should have tracing in there.

Stefan

Patch

diff --git a/virtproxy.c b/virtproxy.c
index 8f18d83..3686c77 100644
--- a/virtproxy.c
+++ b/virtproxy.c
@@ -13,6 +13,23 @@ 
 
 #include "virtproxy.h"
 
+#define DEBUG_VP
+
+#ifdef DEBUG_VP
+#define TRACE(msg, ...) do { \
+    fprintf(stderr, "%s:%s():L%d: " msg "\n", \
+            __FILE__, __FUNCTION__, __LINE__, ## __VA_ARGS__); \
+} while(0)
+#else
+#define TRACE(msg, ...) \
+    do { } while (0)
+#endif
+
+#define LOG(msg, ...) do { \
+    fprintf(stderr, "%s:%s(): " msg "\n", \
+            __FILE__, __FUNCTION__, ## __VA_ARGS__); \
+} while(0)
+
 #define VP_SERVICE_ID_LEN 32    /* max length of service id string */
 #define VP_PKT_DATA_LEN 1024    /* max proxied bytes per VPPacket */
 #define VP_CONN_DATA_LEN 1024   /* max bytes conns can send at a time */