qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Laurent Vivier <lvivier@redhat.com>
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
	qemu-block@nongnu.org, qemu-devel@nongnu.org,
	"Markus Armbruster" <armbru@redhat.com>,
	"Stefan Hajnoczi" <stefanha@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>
Subject: Re: [PULL for-5.2 2/2] scripts/tracetool: silence SystemTap dtrace(1) long long warnings
Date: Tue, 5 Jan 2021 17:39:44 +0000	[thread overview]
Message-ID: <20210105173944.GJ724458@redhat.com> (raw)
In-Reply-To: <702283f5-13c1-00f9-4c83-49c469287483@redhat.com>

On Mon, Jan 04, 2021 at 09:31:19PM +0100, Laurent Vivier wrote:
> On 11/11/2020 16:56, Stefan Hajnoczi wrote:
> > SystemTap's dtrace(1) prints the following warning when it encounters
> > long long arguments:
> > 
> >   Warning: /usr/bin/dtrace:trace/trace-dtrace-hw_virtio.dtrace:76: syntax error near:
> >   probe vhost_vdpa_dev_start
> > 
> >   Warning: Proceeding as if --no-pyparsing was given.
> > 
> > Use the uint64_t and int64_t types, respectively. This works with all
> > host CPU 32- and 64-bit data models (ILP32, LP64, and LLP64) that QEMU
> > supports.
> > 
> > Reported-by: Markus Armbruster <armbru@redhat.com>
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > Message-id: 20201020094043.159935-1-stefanha@redhat.com
> > Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> >  scripts/tracetool/format/d.py | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/scripts/tracetool/format/d.py b/scripts/tracetool/format/d.py
> > index 353722f89c..ebfb714200 100644
> > --- a/scripts/tracetool/format/d.py
> > +++ b/scripts/tracetool/format/d.py
> > @@ -57,6 +57,12 @@ def generate(events, backend, group):
> >                  # Avoid it by changing probe type to signed char * beforehand.
> >                  if type_ == 'int8_t *':
> >                      type_ = 'signed char *'
> > +
> > +            # SystemTap dtrace(1) emits a warning when long long is used
> > +            type_ = type_.replace('unsigned long long', 'uint64_t')
> > +            type_ = type_.replace('signed long long', 'int64_t')
> > +            type_ = type_.replace('long long', 'int64_t')
> > +
> >              if name in RESERVED_WORDS:
> >                  name += '_'
> >              args.append(type_ + ' ' + name)
> > 
> 
> This patch fixes the warning with "d" format, but we have the same kind of problem with
> log-stap format:
> 
>   $ sudo stap -e 'probe begin{printf ("BEGIN")}'  -I .
>   parse error: invalid or missing conversion specifier
>           saw: operator ',' at ./qemu-system-x86_64-log.stp:15118:101
>        source:     printf("%d@%d vhost_vdpa_set_log_base dev: %p base: 0x%x size: %llu
> refcnt: %d fd: %d log: %p\n", pid(), gettimeofday_ns(), dev, base, size, refcnt, fd, log)
> 
>                        ^
> 
>   1 parse error.
>   WARNING: tapset "./qemu-system-x86_64-log.stp" has errors, and will be skipped
>   BEGIN
> 
> This happens because of the "%llu" in the format string.
> 
> I'm wondering if we need to fix all the stap based format or simply replace the "unsigned
> long long" by "uint64_t" in hw/virtio/trace-events?

The problem isn't really the data type, but rather the format string.
systemtap format strings are not quite the same as C format strings.

So we need to re-write %llu into %lu I expect. We already do some
rewriting in log_stap.py but we obviously need a bit more.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



  reply	other threads:[~2021-01-05 17:55 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-11 15:56 [PULL for-5.2 0/2] Tracing patches Stefan Hajnoczi
2020-11-11 15:56 ` [PULL for-5.2 1/2] trace: remove argument from trace_init_file Stefan Hajnoczi
2020-11-11 15:56 ` [PULL for-5.2 2/2] scripts/tracetool: silence SystemTap dtrace(1) long long warnings Stefan Hajnoczi
2021-01-04 20:31   ` Laurent Vivier
2021-01-05 17:39     ` Daniel P. Berrangé [this message]
2020-11-11 21:51 ` [PULL for-5.2 0/2] Tracing patches Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210105173944.GJ724458@redhat.com \
    --to=berrange@redhat.com \
    --cc=armbru@redhat.com \
    --cc=lvivier@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=philmd@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).