Re: [Qemu-devel] [PATCH 1/6] hypertrace: Add documentation

[Lluís Vilanova]

Eric Blake writes:

> On 08/05/2016 10:59 AM, Lluís Vilanova wrote:
>> Signed-off-by: Lluís Vilanova <address@hidden>
>> ---
>> docs/hypertrace.txt |  141 
>> +++++++++++++++++++++++++++++++++++++++++++++++++++
>> docs/tracing.txt    |    3 +
>> 2 files changed, 144 insertions(+)
>> create mode 100644 docs/hypertrace.txt
>> 
>> diff --git a/docs/hypertrace.txt b/docs/hypertrace.txt
>> new file mode 100644
>> index 0000000..4a31bcd
>> --- /dev/null
>> +++ b/docs/hypertrace.txt
>> @@ -0,0 +1,141 @@
>> += Hypertrace channel =

> No explicit copyright means that this document inherits the project
> default of GPLv2+. If that's not what you intended (or to be clear that
> it IS what you intended), you may want to add a copyright and license blurb.

Oh, I just followed what's on other docs. I can add a copyright notice on top.


>> +
>> +The hypertrace channel allows guest code to emit events in QEMU (the host) 
>> using
>> +its tracing infrastructure (see "docs/trace.txt"). This works in both 
>> 'system'
>> +and 'user' modes. That is, hypertrace is to tracing, what hypercalls are to
>> +system calls.
>> +
>> +You can use this to emit an event on both guest and QEMU (host) traces to 
>> easily
>> +synchronize or correlate them. You could also modify you guest's tracing 
>> system
>> +to emit all events through the hypertrace channel, providing a unified and 
>> fully
>> +synchronized trace log. Another use case is timing the performance of guest 
>> code
>> +when optimizing TCG (QEMU traces have a timestamp).

> Do you need to state that this channel should only be opened up to
> trusted guests, as otherwise it represents a security hole that a guest
> can cause a host denial of service by emitting events as fast as possible?

Mmmm, for user-mode ({linux,bsd}-user), that is not a problem, since there is a
single application running. In the softmmu case, it is already "protected" by
the OS filesystem permissions, which won't give you write access to a device by
default (unless you're root). I can certainly clarify that if that's what you
meant.


>> +3. Create a guest application using "qemu-hypertrace.h":
>> +
>> +    cat > /tmp/my-hypertrace.c <<EOF

> I'd suggest <<\EOF here, to make it obvious that we don't want any $foo
> parameter expansion in the heredoc.

Got it.


>> +    #include <stdio.h>
>> +    #include <errno.h>
>> +    #include <stdlib.h>
>> +    #include <string.h>
>> +    #include <qemu-hypertrace.h>
>> +    
>> +    
>> +    int main(int argc, char **argv)
>> +    {
>> +        char *base = NULL;
>> +        if (argc > 1) {
>> +            base = argv[1];
>> +        }
>> +
>> +        /* In 'user' mode this path must be the same we will use to start 
>> QEMU. */
>> +        if (qemu_hypertrace_init(base) != 0) {
>> +            fprintf(stderr, "error: qemu_hypertrace_init: %s\n", 
>> strerror(errno));

> Worth using perror() in this example code?

Yes, thanks.


> Overall looks reasonable.

Thanks,
  Lluis