Stefan Hajnoczi writes: > On Mon, Jul 24, 2017 at 08:02:24PM +0300, Lluís Vilanova wrote: >> This series adds a basic interface to instrument tracing events and control >> their tracing state. >> >> The instrumentation code is dynamically loaded into QEMU (either when it >> starts >> or later using its remote control interfaces). >> >> All events can be instrumented, but the instrumentable events must be >> explicitly >> specified at configure time. >> >> Signed-off-by: Lluís Vilanova <vilan...@ac.upc.edu>
> Hi Lluís, > I'm concerned that the shared library interface will be abused to monkey > patch code into QEMU far beyond instrumentation use cases and/or avoid > the responsibilities of the GPL license. > 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). > Does this alternative sound reasonable to you? You mean to add a user-provided .c file to QEMU at compile-time? (I'm assuming we can keep the "user API" proposed in this series, instead of the one you showed). First, a user might want to provide more than just a .c, so we might have to accept a directory that produces a library that is included into QEMU at link time (a bit more complicated to do portably). Second, the user can still do the same actions you want to shield from, regardless of whether it's a dynamically loaded library (i.e., access any fuction in QEMU). What I propose to do instead is: * For the monkey-patch part, we can limit symbol resolution to the instrumentation API functions when loading the library (e.g., compile QEMU with -fvisibility=hidden). * For the license part, that is a legal issue that can be handled by the API header license, right? (the "public" headers I added are GPL, not LGPL). Besides, if only the intended API is available, I'm not sure if that matters (e.g., we don't care about the license of a dtrace script, since it only has the API provided by QEMU+dtrace). This would be similar to Linux's module support; only selected functions are available to modules, and we could add a license check (e.g., QI_LICENSE("GPL") must be on the instrumentation library or it won't be loaded). Thanks, Lluis