On Tue, Jul 25, 2017 at 05:47:08PM +0300, Lluís Vilanova wrote: > 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).
Proprietary Linux kernel modules are controversial and some still consider them license violations - especially when an "open source" shim module is used to interface between proprietary code and the kernel. What is the use case for this instrumentation? Stefan
signature.asc
Description: PGP signature