[Qemu-devel] [PATCH 1/4] trace: Use portable format strings

[Qemu-devel] [PATCH 1/4] trace: Use portable format strings

[Stefan Hajnoczi]

It is not portable to use "%ld" for int64_t because int64_t may have
type long on 64-bit platforms and long long on 32-bit platforms.  Use
the standard library PRId64 macros to keep format strings portable.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 docs/tracing.txt |    4 ++++
 trace-events     |    4 ++--
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/docs/tracing.txt b/docs/tracing.txt
index ae01ff1..5504850 100644
--- a/docs/tracing.txt
+++ b/docs/tracing.txt
@@ -72,6 +72,10 @@ Trace events should use types as follows:
  * For everything else, use primitive scalar types (char, int, long) with the
    appropriate signedness.
 
+Format strings should reflect the types defined in the trace event.  Take
+special care to use PRId64 and PRIu64 for int64_t and uint64_t types,
+respectively.  This ensures portability between 32- and 64-bit platforms.
+
 === Hints for adding new trace events ===
 
 1. Trace state changes in the code.  Interesting points in the code usually
diff --git a/trace-events b/trace-events
index f32c83f..b43317e 100644
--- a/trace-events
+++ b/trace-events
@@ -55,10 +55,10 @@ disable bdrv_aio_multiwrite_latefail(void *mcb, int i) "mcb %p i %d"
 # hw/virtio-blk.c
 disable virtio_blk_req_complete(void *req, int status) "req %p status %d"
 disable virtio_blk_rw_complete(void *req, int ret) "req %p ret %d"
-disable virtio_blk_handle_write(void *req, unsigned long sector, unsigned long nsectors) "req %p sector %lu nsectors %lu"
+disable virtio_blk_handle_write(void *req, uint64_t sector, size_t nsectors) "req %p sector %"PRIu64" nsectors %zu"
 
 # posix-aio-compat.c
-disable paio_submit(void *acb, void *opaque, unsigned long sector_num, unsigned long nb_sectors, unsigned long type) "acb %p opaque %p sector_num %lu nb_sectors %lu type %lu"
+disable paio_submit(void *acb, void *opaque, int64_t sector_num, int nb_sectors, int type) "acb %p opaque %p sector_num %"PRId64" nb_sectors %d type %d"
 
 # ioport.c
 disable cpu_in(unsigned int addr, unsigned int val) "addr %#x value %u"
-- 
1.7.1

[Qemu-devel] [PATCH 2/4] trace: Don't strip lines containing '#' arbitrarily

[Stefan Hajnoczi]

Although comment lines must be skipped, the '#' character can occur in
valid format strings.  Be more careful when checking for comments.
Leave comments at the end of the line where they will not interfere with
other processing.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 tracetool |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/tracetool b/tracetool
index 534cc70..63e3e29 100755
--- a/tracetool
+++ b/tracetool
@@ -318,8 +318,7 @@ convert()
 
     while read -r str; do
         # Skip comments and empty lines
-        str=${str%%#*}
-        test -z "$str" && continue
+        test -z "${str%%#*}" && continue
 
         # Process the line.  The nop backend handles disabled lines.
         disable=${str%%disable *}
-- 
1.7.1

[Qemu-devel] [PATCH 3/4] trace: Use TP_PROTO() and TP_ARGS() for LTTng UST

[Stefan Hajnoczi]

The LTTng UserSpace Tracer formerly used TPPROTO() and TPARGS() instead
of TP_PROTO() and TP_ARGS() like the kernel uses.  This has been changed
so QEMU needs to follow.

I am not aware of a graceful way of making the transition but since no
one complained that the UST build is broken, it should be fine to just
switch over without compatibility for old UST headers.  The newer UST
headers are shipping in distro packages so it is realistic to make this
change now.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 tracetool |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tracetool b/tracetool
index 63e3e29..7010858 100755
--- a/tracetool
+++ b/tracetool
@@ -250,7 +250,7 @@ linetoh_ust()
     argnames=$(get_argnames "$1")
 
     cat <<EOF
-DECLARE_TRACE(ust_$name, TPPROTO($args), TPARGS($argnames));
+DECLARE_TRACE(ust_$name, TP_PROTO($args), TP_ARGS($argnames));
 #define trace_$name trace_ust_$name
 EOF
 }
-- 
1.7.1

[Qemu-devel] [PATCH 4/4] trace: Trace bdrv_aio_{readv,writev}

[Stefan Hajnoczi]

Observing block layer aio readv/writev operations is useful for
debugging image formats or understanding guest disk I/O patterns.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 block.c      |    4 ++++
 trace-events |    2 ++
 2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/block.c b/block.c
index ebbc376..a19374d 100644
--- a/block.c
+++ b/block.c
@@ -1983,6 +1983,8 @@ BlockDriverAIOCB *bdrv_aio_readv(BlockDriverState *bs, int64_t sector_num,
     BlockDriver *drv = bs->drv;
     BlockDriverAIOCB *ret;
 
+    trace_bdrv_aio_readv(bs, sector_num, nb_sectors, opaque);
+
     if (!drv)
         return NULL;
     if (bdrv_check_request(bs, sector_num, nb_sectors))
@@ -2007,6 +2009,8 @@ BlockDriverAIOCB *bdrv_aio_writev(BlockDriverState *bs, int64_t sector_num,
     BlockDriver *drv = bs->drv;
     BlockDriverAIOCB *ret;
 
+    trace_bdrv_aio_writev(bs, sector_num, nb_sectors, opaque);
+
     if (!drv)
         return NULL;
     if (bs->read_only)
diff --git a/trace-events b/trace-events
index b43317e..4300178 100644
--- a/trace-events
+++ b/trace-events
@@ -51,6 +51,8 @@ disable multiwrite_cb(void *mcb, int ret) "mcb %p ret %d"
 disable bdrv_aio_multiwrite(void *mcb, int num_callbacks, int num_reqs) "mcb %p num_callbacks %d num_reqs %d"
 disable bdrv_aio_multiwrite_earlyfail(void *mcb) "mcb %p"
 disable bdrv_aio_multiwrite_latefail(void *mcb, int i) "mcb %p i %d"
+disable bdrv_aio_readv(void *bs, int64_t sector_num, int nb_sectors, void *opaque) "bs %p sector_num %"PRId64" nb_sectors %d opaque %p"
+disable bdrv_aio_writev(void *bs, int64_t sector_num, int nb_sectors, void *opaque) "bs %p sector_num %"PRId64" nb_sectors %d opaque %p"
 
 # hw/virtio-blk.c
 disable virtio_blk_req_complete(void *req, int status) "req %p status %d"
-- 
1.7.1

[Qemu-devel] Re: [PATCH 1/4] trace: Use portable format strings

[Blue Swirl]

Thanks, applied all.

On Tue, Oct 5, 2010 at 1:28 PM, Stefan Hajnoczi
<stefanha@linux.vnet.ibm.com> wrote:
> It is not portable to use "%ld" for int64_t because int64_t may have
> type long on 64-bit platforms and long long on 32-bit platforms.  Use
> the standard library PRId64 macros to keep format strings portable.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> ---
>  docs/tracing.txt |    4 ++++
>  trace-events     |    4 ++--
>  2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/docs/tracing.txt b/docs/tracing.txt
> index ae01ff1..5504850 100644
> --- a/docs/tracing.txt
> +++ b/docs/tracing.txt
> @@ -72,6 +72,10 @@ Trace events should use types as follows:
>  * For everything else, use primitive scalar types (char, int, long) with the
>    appropriate signedness.
>
> +Format strings should reflect the types defined in the trace event.  Take
> +special care to use PRId64 and PRIu64 for int64_t and uint64_t types,
> +respectively.  This ensures portability between 32- and 64-bit platforms.
> +
>  === Hints for adding new trace events ===
>
>  1. Trace state changes in the code.  Interesting points in the code usually
> diff --git a/trace-events b/trace-events
> index f32c83f..b43317e 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -55,10 +55,10 @@ disable bdrv_aio_multiwrite_latefail(void *mcb, int i) "mcb %p i %d"
>  # hw/virtio-blk.c
>  disable virtio_blk_req_complete(void *req, int status) "req %p status %d"
>  disable virtio_blk_rw_complete(void *req, int ret) "req %p ret %d"
> -disable virtio_blk_handle_write(void *req, unsigned long sector, unsigned long nsectors) "req %p sector %lu nsectors %lu"
> +disable virtio_blk_handle_write(void *req, uint64_t sector, size_t nsectors) "req %p sector %"PRIu64" nsectors %zu"
>
>  # posix-aio-compat.c
> -disable paio_submit(void *acb, void *opaque, unsigned long sector_num, unsigned long nb_sectors, unsigned long type) "acb %p opaque %p sector_num %lu nb_sectors %lu type %lu"
> +disable paio_submit(void *acb, void *opaque, int64_t sector_num, int nb_sectors, int type) "acb %p opaque %p sector_num %"PRId64" nb_sectors %d type %d"
>
>  # ioport.c
>  disable cpu_in(unsigned int addr, unsigned int val) "addr %#x value %u"
> --
> 1.7.1
>
>