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

[Eric Blake]

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.

> +
> +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?


> +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.

> +    #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?

Overall looks reasonable.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature