From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:48793)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <vilanova@ac.upc.edu>) id 1bWkCv-0004nL-Up
	for qemu-devel@nongnu.org; Mon, 08 Aug 2016 09:03:07 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <vilanova@ac.upc.edu>) id 1bWkCr-0003d7-Om
	for qemu-devel@nongnu.org; Mon, 08 Aug 2016 09:03:04 -0400
Received: from roura.ac.upc.edu ([147.83.33.10]:39204 helo=roura.ac.upc.es)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <vilanova@ac.upc.edu>) id 1bWkCr-0003bf-D5
	for qemu-devel@nongnu.org; Mon, 08 Aug 2016 09:03:01 -0400
From: =?utf-8?Q?Llu=C3=ADs_Vilanova?= <vilanova@ac.upc.edu>
References: <147041636348.2523.2954972609232949598.stgit@fimbulvetr.bsc.es>
	<147041636895.2523.17410454408859217963.stgit@fimbulvetr.bsc.es>
	<57A4CA1D.9040004@redhat.com>
Date: Mon, 08 Aug 2016 15:02:54 +0200
In-Reply-To: <57A4CA1D.9040004@redhat.com> (Eric Blake's message of "Fri, 5
	Aug 2016 11:17:17 -0600")
Message-ID: <87r39z8lc1.fsf@fimbulvetr.bsc.es>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [Qemu-devel] [PATCH 1/6] hypertrace: Add documentation
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel/>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Eric Blake <eblake@redhat.com>
Cc: qemu-devel@nongnu.org, Stefan Hajnoczi <stefanha@redhat.com>

Eric Blake writes:

> On 08/05/2016 10:59 AM, Llu=C3=ADs Vilanova wrote:
>> Signed-off-by: Llu=C3=ADs Vilanova <vilanova@ac.upc.edu>
>> ---
>> docs/hypertrace.txt |  141 +++++++++++++++++++++++++++++++++++++++++++++=
++++++
>> docs/tracing.txt    |    3 +
>> 2 files changed, 144 insertions(+)
>> create mode 100644 docs/hypertrace.txt
>>=20
>> 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 @@
>> +=3D Hypertrace channel =3D

> 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 blu=
rb.

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


>> +
>> +The hypertrace channel allows guest code to emit events in QEMU (the ho=
st) using
>> +its tracing infrastructure (see "docs/trace.txt"). This works in both '=
system'
>> +and 'user' modes. That is, hypertrace is to tracing, what hypercalls ar=
e 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 tracin=
g 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 g=
uest 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 devic=
e by
default (unless you're root). I can certainly clarify that if that's what y=
ou
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>
>> +=20=20=20=20
>> +=20=20=20=20
>> +    int main(int argc, char **argv)
>> +    {
>> +        char *base =3D NULL;
>> +        if (argc > 1) {
>> +            base =3D argv[1];
>> +        }
>> +
>> +        /* In 'user' mode this path must be the same we will use to sta=
rt QEMU. */
>> +        if (qemu_hypertrace_init(base) !=3D 0) {
>> +            fprintf(stderr, "error: qemu_hypertrace_init: %s\n", strerr=
or(errno));

> Worth using perror() in this example code?

Yes, thanks.


> Overall looks reasonable.

Thanks,
  Lluis