Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation

From: Peter Maydell <peter.maydell@linaro.org>
To: Stefan Hajnoczi <stefanha@gmail.com>
Cc: "Lluís Vilanova" <vilanova@ac.upc.edu>,
	"Emilio G. Cota" <cota@braap.org>,
	"QEMU Developers" <qemu-devel@nongnu.org>,
	"Stefan Hajnoczi" <stefanha@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation
Date: Tue, 25 Jul 2017 14:30:06 +0100	[thread overview]
Message-ID: <CAFEAcA_pmFYZ_y9E5D0c70G2oA62vb+KBXnxbp_Sg39kuNeREA@mail.gmail.com> (raw)
In-Reply-To: <20170725131931.GC23343@stefanha-x1.localdomain>

On 25 July 2017 at 14:19, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> Instead I suggest adding a trace backend generates calls to registered
> "callback" functions:
>
>   $ cat >my-instrumentation.c
>   #include "trace/control.h"
>
>   static void my_cpu_in(unsigned int addr, char size, unsigned int val)
>   {
>       printf("my_cpu_in\n");
>   }
>
>   static void my_init(void)
>   {
>       trace_register_event_callback("cpu_in", my_cpu_in);
>       trace_enable_events("cpu_in");
>   }
>   trace_init(my_init);
>
>   $ ./configure --enable-trace-backends=log,callback && make -j4
>
> This is still a clean interface that allows instrumentation code to be
> kept separate from the trace event call sites.
>
> The instrumentation code gets compiled into QEMU, but that shouldn't be
> a huge burden since QEMU's Makefiles only recompile changed source
> files (only the first build is slow).

Is your proposal that my-instrumentation.c gets compiled into
QEMU at this point? That doesn't seem like a great idea to
me, because it means you can only use this tracing if you
build QEMU yourself, and distros won't enable it and so
lots of our users won't have convenient access to it.
I'd much rather see a cleanly designed plugin interface
(which we should be able to implement in a manner that doesn't
let the plugin monkey patch arbitrary parts of QEMU beyond
what can already be achieved via LD_PRELOAD).

thanks
-- PMM