qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
Cc: qemu-devel@nongnu.org, qemu-rust@nongnu.org,
	"Philippe Mathieu-Daudé" <philmd@linaro.org>,
	alex.bennee@linaro.org
Subject: Re: [PATCH RFC 0/5] rust: implement tracing
Date: Tue, 5 Aug 2025 18:54:30 +0100	[thread overview]
Message-ID: <aJJFVgkt7R49ZwIF@redhat.com> (raw)
In-Reply-To: <CAAjaMXZ0Dho+5FSyc4zkOZXZT6UvHtjgmpj6_0QZQcH_AD9=VQ@mail.gmail.com>

On Tue, Aug 05, 2025 at 07:47:39PM +0300, Manos Pitsidianakis wrote:
> On Tue, Aug 5, 2025 at 7:43 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > On Tue, Aug 05, 2025 at 07:25:39PM +0300, Manos Pitsidianakis wrote:
> > > On Tue, Aug 5, 2025 at 7:05 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
> > > >
> > > > On Mon, Aug 04, 2025 at 04:47:13PM +0300, Manos Pitsidianakis wrote:
> > > > > This RFC series contains some simple patches I've been sitting on for
> > > > > some months to allow tracing in rust devices in a similar matter to C,
> > > > > only it's done via a proc-macro codegen instead of using tracetool
> > > > > script or equivalent.
> > > >
> > > > IIUC, this series is only emitting the traces events via the
> > > > qemu_log function, and so feels like it is missing the benefit
> > > > of tracing, vs the traditional logging framework.
> > > >
> > > > In our RHEL & Fedora distro builds we disable the log backend
> > > > and enable dtrace, so that we have fully dynamic tracing and
> > > > observability across the kernel, qemu, libvirt and other
> > > > components with dtrace integration.
> > >
> > > Hi Daniel,
> > >
> > > Thanks for the insight! Do you have any points where I should look at
> > > the trace implementation for how the different backends are supported?
> > >
> > > So I think there's already work in progress to support proper tracing
> > > for Rust, I only sent this as a temporary fixup to provide some kind
> > > of parity between C and Rust implementations until a proper, better
> > > solution is available that can replace it.
> >
> > Can the rust code not easily consume the existing functions in the
> > trace.h files generated for the C code as a short-term solution ?
> >
> > It would not benefit from the code inlining in the same way as C
> > would, but it would at least give feature parity for tracing with
> > all the trace backends are available.
> >
> > Then, we can look at optimizing with a pure rust impl of some
> > backends at a later date, to regain what we lost from lack of
> > inlining ?
> 
> It can, but we'd need to add extra intermediate steps to convert the
> trace headers into Rust equivalent code, so it's not ideal.
> 
> I tried to generate code exactly like the generated trace headers
> though, so I'm not sure what is missing to be honest (hence my
> previous email question). The generated code generates TraceEvents and
> registers them with trace_event_register_group. What else is missing
> to support e.g. dtrace?

'trace_event_register_group' is essentially irrelevant for the
fully dynamic trace backends like dtrace - that's only used for
the backends whose output is controlled by QEMU monitor commands
/ command line arguments.

In the dtrace case the binary gets instructions which are a squence
of nops normally, and dtrace tool gets the kernel to live patch the
binary at runtime to put in a jump for any probes that are being
watched.

Take a look at the generated files <build-dir>/trace/trace-*.h when
using the different '--enable-trace-backends=...' options.

eg taking the trace-crypto.h header, with 'log' backend we see it
emits

   if (trace_event_get_state(TRACE_QCRYPTO_TLS_SESSION_CHECK_CREDS) && qemu_loglevel_mask(LOG_TRACE)) {
#line 23 "../crypto/trace-events"
        qemu_log("qcrypto_tls_session_check_creds " "TLS session check creds session=%p status=%s" "\n", session, status);
#line 372 "trace/trace-crypto.h"
    }

but with dtrace it emits

    QEMU_QCRYPTO_TLS_SESSION_CHECK_CREDS(session, status);

which is a referencing a macro created by the external 'dtrace' binary,
which in the Linux case ends up looking like

  #if defined STAP_SDT_V1
  #define QEMU_QCRYPTO_TLS_SESSION_CHECK_CREDS_ENABLED() __builtin_expect (qcrypto_tls_session_check_creds_semaphore, 0)
  #define qemu_qcrypto_tls_session_check_creds_semaphore qcrypto_tls_session_check_creds_semaphore
  #else
  #define QEMU_QCRYPTO_TLS_SESSION_CHECK_CREDS_ENABLED() __builtin_expect (qemu_qcrypto_tls_session_check_creds_semaphore, 0)
  #endif
  __extension__ extern unsigned short qemu_qcrypto_tls_session_check_creds_semaphore __attribute__ ((unused)) __attribute__ ((section (".probes")));
  #define QEMU_QCRYPTO_TLS_SESSION_CHECK_CREDS(arg1, arg2) \
  DTRACE_PROBE2 (qemu, qcrypto_tls_session_check_creds, arg1, arg2)

you can end up enabling multiple trace backends concurrently too.

If you're thinking this is all rather complicated, you'd be right,
which is why for initial feature parity I figured the simplest is
likely to just wrap the existing QEMU inline probe function, so
Rust doesn't need to know about the different backends... yet...

FWIW, the original DTrace authors created a Rust crate with native
rust integration of dynamic probes.

  https://github.com/oxidecomputer/usdt

I think that (somehow) we probably want to integrate that with QEMU
and its tracetool.

With 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:[~2025-08-05 17:55 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-04 13:47 [PATCH RFC 0/5] rust: implement tracing Manos Pitsidianakis
2025-08-04 13:47 ` [PATCH RFC 1/5] rust/bindings: add trace headers Manos Pitsidianakis
2025-08-04 13:47 ` [PATCH RFC 2/5] rust/qemu-api/log: add Log::Trace variant Manos Pitsidianakis
2025-08-04 13:47 ` [PATCH RFC 3/5] rust/qemu-api-macros: Add #[trace_events] macro Manos Pitsidianakis
2025-08-04 13:47 ` [PATCH RFC 4/5] rust/pl011: impl Copy, Clone for RegisterOffset Manos Pitsidianakis
2025-08-04 13:47 ` [PATCH RFC 5/5] rust/pl011: add trace events Manos Pitsidianakis
2025-08-05 16:04 ` [PATCH RFC 0/5] rust: implement tracing Daniel P. Berrangé
2025-08-05 16:25   ` Manos Pitsidianakis
2025-08-05 16:43     ` Daniel P. Berrangé
2025-08-05 16:47       ` Manos Pitsidianakis
2025-08-05 17:54         ` Daniel P. Berrangé [this message]
2025-08-05 20:06           ` Manos Pitsidianakis
2025-08-06  9:02             ` Paolo Bonzini
2025-08-06  9:21               ` Daniel P. Berrangé
2025-08-06  9:34                 ` Manos Pitsidianakis
2025-08-06  9:36               ` Manos Pitsidianakis

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=aJJFVgkt7R49ZwIF@redhat.com \
    --to=berrange@redhat.com \
    --cc=alex.bennee@linaro.org \
    --cc=manos.pitsidianakis@linaro.org \
    --cc=philmd@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-rust@nongnu.org \
    /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).