Stefan Hajnoczi writes:
> 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?
As I said, we can require instrumentation libraries to be GPL (and nothing
else), so no proprietary code is possible without a license violation (then
we're on the same field as if someone ships a modified QEMU binary, which
technically is just as doable).
As for use-cases, see Peter's email and my response.
Cheers,
Lluis
