Re: [Qemu-devel] [PATCH 0/6] hypertrace: Lightweight guest-to-QEMU trace channel

[Steven Rostedt]

On Fri, 19 Aug 2016 11:02:07 +0100
Stefan Hajnoczi <stefa...@redhat.com> wrote:


> > [1] 
> > https://lcccna2016.sched.org/event/7JWL/trace-cmd-virt-server-a-status-update-steven-rostedt-red-hat?iframe=no=i:100;=yes=no
> >   
> 
> Cool, that's very interesting!  I'll try to attend.

Good, see you then!

> 
> I guess LinuxCon will post a video of your talk and the slides will be
> available after the talk so people not at LinuxCon will be able to check
> it out too.

Hmm, IIRC the LinuxCon talks are seldom recorded. But the slides will be
available.

-- Steve

Re: [Qemu-devel] [PATCH 0/6] hypertrace: Lightweight guest-to-QEMU trace channel

[Steven Rostedt]

On Thu, 18 Aug 2016 11:54:24 +0100
Stefan Hajnoczi <stefa...@gmail.com> wrote:

> Steven is working on a host/guest solution for trace-cmd.  It is also
> asynchronous.  No new paravirt hardware is needed and it makes me wonder
> whether the hypertrace PCI device is trying to solve the problem at the
> wrong layer.

Yes, and I'm currently working on it again. In fact, if any of you will
be at LinuxCon in Toronto next week, I'll be presenting[1] what I have
and what I plan to complete in the near future.

-- Steve

[1] 
https://lcccna2016.sched.org/event/7JWL/trace-cmd-virt-server-a-status-update-steven-rostedt-red-hat?iframe=no=i:100;=yes=no

Re: [Qemu-devel] [PATCH 6/7] qemu: Implement virtio-pstore device

[Steven Rostedt]

On Thu, 28 Jul 2016 14:39:53 +0900
Namhyung Kim  wrote:

> Well, I dont' know.  As you know, the kernel oops dump is already sent
> to serial device but it's rather slow.  As I wrote in the cover
> letter, enabling ftrace_dump_on_oops makes it even worse..  Also
> pstore saves the (compressed) binary data so I thought it'd be better
> to have a dedicated IO channel.

BTW, I agree with this. It is better to have a quick way to grab the
ftrace buffers when a crash happens, as serial is excruciatingly slow.
Although, currently I still use kexec/kdump, but as Namhyung said, it
depends on crash being up to date. I tend to be sending in updates
every time I have to use it.

-- Steve

Re: [Qemu-devel] [RFC] host and guest kernel trace merging

[Steven Rostedt]

On Mon, 7 Mar 2016 09:10:10 -0700
Eric Blake <ebl...@redhat.com> wrote:

> On 03/07/2016 08:49 AM, Steven Rostedt wrote:
> > On Mon, 7 Mar 2016 15:17:05 +
> > Stefan Hajnoczi <stefa...@redhat.com> wrote:
> > 
> >   
> >> qemu-guest-agent runs inside the guest and replies to RPC commands from
> >> the host.  It is used for backups, shutdown, network configuration, etc.
> >> From time to time people have wanted the ability to execute an arbitrary
> >> command inside the guest and return the output.  This functionality has
> >> never been merged, probably for the security reason.  
> > 
> > How's the connection set up. That is, how does it know the commands are
> > coming from the host? And how does it know that the commands from the
> > host is from a trusted source? If the host is compromised, is there
> > anything keeping an intruder from controlling the guest?  
> 
> qemu-guest-agent uses a virtio channel, so only the host can be driving
> that channel.  But how can a guest know that it trusts the host? It
> can't.  A compromised host implicitly compromises all guests, and that's
> always been the case.  At least qemu-guest-agent doesn't make the window
> any larger.
> 

I should have been a bit more clear about what I meant by "host is
compromised". I should have asked, what about untrusted tasks on the
host. Is the channel protected where only admin users can access it?

Of course, one of my concerns with the trace-cmd server is that it may
require a network connection, because doesn't a virtio channel require
to be initialized at boot up? Where the host must have an active
listener when the guest starts? Or am I thinking about something else.

-- Steve

Re: [Qemu-devel] [RFC] host and guest kernel trace merging

[Steven Rostedt]

On Mon, 7 Mar 2016 15:17:05 +
Stefan Hajnoczi  wrote:


> qemu-guest-agent runs inside the guest and replies to RPC commands from
> the host.  It is used for backups, shutdown, network configuration, etc.
> From time to time people have wanted the ability to execute an arbitrary
> command inside the guest and return the output.  This functionality has
> never been merged, probably for the security reason.

How's the connection set up. That is, how does it know the commands are
coming from the host? And how does it know that the commands from the
host is from a trusted source? If the host is compromised, is there
anything keeping an intruder from controlling the guest?

> 
> A tracing server that runs inside the guest is comparable to
> qemu-guest-agent.  As long as the tracing server requires manual
> commands to start it and does not run by default, then I think the
> security issue can be kept at bay.  It's a powerful tool that requires
> explicit guest administrator action to enable.

This isn't something I would expect to be started by default. My worry
is that once it is started, then you open up a connection that can be
attached by pretty much anyone. We could make a network socket that
only communicates with the host, but even that worries me. I'm worried
that the host may have actors that might compromise the system.

-- Steve

Re: [Qemu-devel] [RFC] host and guest kernel trace merging

[Steven Rostedt]

On Fri, 4 Mar 2016 11:19:33 +
Stefan Hajnoczi  wrote:


> >  2. Build a trace-cmd-server
> > 
> > Which does the following:
> > 
> >   1. Receive trace-cmd record commands from a guest,
> >  to be performed in the host  
> 
> Sometimes the opposite is desirable: the host controls tracing inside
> the guest.  Any thoughts on this use case?

My idea for a trace-cmd server, is to have a --client operation, for
running on the guest.

 trace-cmd server --client 

The connection will be some socket, either network or something
directly attached to the host.

Then on the host, we can have

  trace-cmd server --connect 

Where the server will create a connection to the guest.

And then, you could run on the host:

  trace-cmd record  --connect  

And this will start recording host events, and then connect to the
local server that connects to the guest(s) and that will start tracing
on the guest as well.

Then events on the guest will be passed to the host server.

Something like this is my idea. We can work out the details on the best
way to get things working. We may be able to eliminate the host server
middle man. But I envision that we need a trace-cmd server running on
the guest to start off the commands.

The problem I have with the guest server, and something that we may be
able to fix later on, but should always keep it in the back of our
minds, is the security issue. For this to work, the guest server needs
to run as root. It will have an open socket (network or to host), that
will enable tracing on the guest. There needs to be some sort of
verification on that connection to prevent anyone from connecting to it.

In the protocol for the connection between guest and host, I'll
currently add a "security" feature, that will allow the guest to tell
whomever is connecting to it, what type of security feature it wants.
For now it will be TRACE_CMD_NO_SECURITY. But that will have to change
in the future.

-- Steve


> 
> >   2. Read the TSC offset for vCPUs being traced
> > 
> >   3. Read the CPU frequency for --ts2secs
> > 
> >   4. Receive the guest.dat from the guest and store it together
> >  with the host's
> > 
> > I'd suggest the default communication between guest and
> > host be via networking. Then we add vsock support when it's
> > available.  

Re: [Qemu-devel] [PATCH 2/5] trace-cmd: Use tracing directory to count CPUs

[Steven Rostedt]

On Thu, 2012-08-23 at 12:00 +0900, Masami Hiramatsu wrote:
 (2012/08/23 11:01), Masami Hiramatsu wrote:
  (2012/08/22 22:41), Steven Rostedt wrote:
  On Wed, 2012-08-22 at 17:43 +0900, Yoshihiro YUNOMAE wrote:
  From: Masami Hiramatsu masami.hiramatsu...@hitachi.com
 
  Count debugfs/tracing/per_cpu/cpu* to determine the
  number of CPUs.
 
  I'm curious, do you find that sysconf doesn't return the # of CPUs the
  system has?
  
  No, sysconf returns the number of hosts CPUs, not guests.
  
  I've had boxes where the per_cpu/cpu* had more cpus than the
  box actually holds. But this was a bug in the kernel, not the tool. This
  change log needs to have rational instead of just explaining what the
  patch does.
  
  Ah, I see. Hmm, then this should be enabled by a command line
  option or an environment variable.
 
 Oops, I misunderstood. I'll add more comment for why this
 should be tried instead of sysconf.

And now that I understand why you are doing this, why not only do this
if the TRACE_AGENT or DEBUG_TRACING_DIR is defined. That is, if we are
doing it against a bare metal system, then sysconf should suffice, but
if we are tracing against a guest, then it should use the tracing
directory to determine the buffers.

We could add options to override this, but I would think the default
should just Do The Right Thing(tm).

-- Steve

Re: [Qemu-devel] [PATCH 1/5] trace-cmd: Use TRACE_DIR envrionment variable if defined

[Steven Rostedt]

On Wed, 2012-08-22 at 17:43 +0900, Yoshihiro YUNOMAE wrote:
 From: Masami Hiramatsu masami.hiramatsu...@hitachi.com
 
 Use TRACE_DIR environment variable for setting

TRACING_DIR would be better, as we are searching for /debug/tracing and
not /debug/trace. Perhaps DEBUG_TRACING_DIR would be even better as to
be less of a generic term.

-- Steve

 debugfs/tracing directory if defined. This is
 for controlling guest(or remote) ftrace.
 
 Signed-off-by: Masami Hiramatsu masami.hiramatsu...@hitachi.com
 Signed-off-by: Yoshihiro YUNOMAE yoshihiro.yunomae...@hitachi.com
 ---
 
  trace-util.c |9 +
  1 files changed, 9 insertions(+), 0 deletions(-)
 
 diff --git a/trace-util.c b/trace-util.c
 index e128188..d5a3eb4 100644
 --- a/trace-util.c
 +++ b/trace-util.c
 @@ -311,6 +311,15 @@ char *tracecmd_find_tracing_dir(void)
   char type[100];
   FILE *fp;
   
 + tracing_dir = getenv(TRACE_DIR);
 + if (tracing_dir) {
 + tracing_dir = strdup(tracing_dir);
 + if (!tracing_dir)
 + die(malloc);
 + warning(Use environmental tracing directory: %s\n, 
 tracing_dir);
 + return tracing_dir;
 + }
 +
   if ((fp = fopen(/proc/mounts,r)) == NULL) {
   warning(Can't open /proc/mounts for read);
   return NULL;
 

Re: [Qemu-devel] [PATCH 2/5] trace-cmd: Use tracing directory to count CPUs

[Steven Rostedt]

On Wed, 2012-08-22 at 17:43 +0900, Yoshihiro YUNOMAE wrote:
 From: Masami Hiramatsu masami.hiramatsu...@hitachi.com
 
 Count debugfs/tracing/per_cpu/cpu* to determine the
 number of CPUs.

I'm curious, do you find that sysconf doesn't return the # of CPUs the
system has? I've had boxes where the per_cpu/cpu* had more cpus than the
box actually holds. But this was a bug in the kernel, not the tool. This
change log needs to have rational instead of just explaining what the
patch does.

Thanks,

-- Steve

 
 Signed-off-by: Masami Hiramatsu masami.hiramatsu...@hitachi.com
 Signed-off-by: Yoshihiro YUNOMAE yoshihiro.yunomae...@hitachi.com
 ---

Re: [Qemu-devel] [PATCH 3/5] trace-cmd: Support trace-agent of virtio-trace

[Steven Rostedt]

On Wed, 2012-08-22 at 17:43 +0900, Yoshihiro YUNOMAE wrote:
 Add read path and control path to use trace-agent of virtio-trace.
 When we use trace-agent, trace-cmd will be used as follows:
   # AGENT_READ_DIR=/tmp/virtio-trace/tracing \
 AGENT_CTL=/tmp/virtio-trace/agent-ctl-path.in \
 TRACING_DIR=/tmp/virtio-trace/debugfs/tracing \\

Ha! You used TRACING_DIR but patch one introduces TRACE_DIR. Lets
change this to DEBUG_TRACING_DIR instead anyway.

Also, I don't like the generic environment variables. Perhaps
VIRTIO_TRACE_DIR, or AGENT_TRACE_DIR and AGENT_TRACE_CTL. Lets try to
keep the environment namespace sparse.

 trace-cmd record -e sched:*
 Here, AGENT_READ_DIR is the path for a reading directory of virtio-trace,
 AGENT_CTL is a control path of trace-agent, and TRACING_DIR is a debugfs path
 of a guest.
 
 Signed-off-by: Yoshihiro YUNOMAE yoshihiro.yunomae...@hitachi.com
 ---
 
  trace-cmd.h  |1 +
  trace-recorder.c |   57 
 +-
  trace-util.c |   18 +
  3 files changed, 75 insertions(+), 1 deletions(-)
 
 diff --git a/trace-cmd.h b/trace-cmd.h
 index f904dc5..75506ed 100644
 --- a/trace-cmd.h
 +++ b/trace-cmd.h
 @@ -72,6 +72,7 @@ static inline int tracecmd_host_bigendian(void)
  }
  
  char *tracecmd_find_tracing_dir(void);
 +char *guest_agent_tracing_read_dir(void);
  
  /* --- Opening and Reading the trace.dat file --- */
  
 diff --git a/trace-recorder.c b/trace-recorder.c
 index 215affc..3b750e9 100644
 --- a/trace-recorder.c
 +++ b/trace-recorder.c
 @@ -33,6 +33,7 @@
  #include unistd.h
  #include ctype.h
  #include errno.h
 +#include stdbool.h
  
  #include trace-cmd.h
  
 @@ -43,6 +44,8 @@ struct tracecmd_recorder {
   int page_size;
   int cpu;
   int stop;
 + int ctl_fd;
 + boolagent_existing;

Thanks for the reminder. I need to convert a lot to use 'bool' instead.

  };
  
  void tracecmd_free_recorder(struct tracecmd_recorder *recorder)
 @@ -59,11 +62,29 @@ void tracecmd_free_recorder(struct tracecmd_recorder 
 *recorder)
   free(recorder);
  }
  
 +static char *use_trace_agent_dir(char *ctl_path,
 + struct tracecmd_recorder *recorder)
 +{
 + ctl_path = strdup(ctl_path);
 + if (!ctl_path)
 + die(malloc);
 + warning(Use environmental control path: %s\n, ctl_path);

s/Use/Using/

-- Steve

 +
 + recorder-ctl_fd = open(ctl_path, O_WRONLY);
 + if (recorder-ctl_fd  0)
 + return NULL;
 +
 + recorder-agent_existing = true;
 +
 + return guest_agent_tracing_read_dir();
 +}
 +

Re: [Qemu-devel] [RFC PATCH 2/6] virtio/console: Add a failback for unstealable pipe buffer

[Steven Rostedt]

On Thu, 2012-08-09 at 18:24 +0900, Masami Hiramatsu wrote:

 Yeah, it is really easy to fix that.
 But out of curiosity, would that be really a problem?
 I guess that host can access any guest page if need. If that
 is right, is that really insecure to leak randomly allocated
 unused page to the host?

Yeah, it's like protecting userspace pages from the kernel ;-)

-- Steve

Re: [Qemu-devel] [RFC PATCH 4/6] ftrace: Allow stealing pages from pipe buffer

[Steven Rostedt]

On Tue, 2012-07-24 at 11:37 +0900, Yoshihiro YUNOMAE wrote:
 From: Masami Hiramatsu masami.hiramatsu...@hitachi.com
 
 Use generic steal operation on pipe buffer to allow stealing
 ring buffer's read page from pipe buffer.
 
 Note that this could reduce the performance of splice on the
 splice_write side operation without affinity setting.
 Since the ring buffer's read pages are allocated on the
 tracing-node, but the splice user does not always execute
 splice write side operation on the same node. In this case,
 the page will be accessed from the another node.
 Thus, it is strongly recommended to assign the splicing
 thread to corresponding node.
 
 Signed-off-by: Masami Hiramatsu masami.hiramatsu...@hitachi.com

Acked-by: Steven Rostedt rost...@goodmis.org

-- Steve


 Cc: Frederic Weisbecker fweis...@gmail.com
 Cc: Ingo Molnar mi...@redhat.com
 ---
 
  kernel/trace/trace.c |8 +---
  1 files changed, 1 insertions(+), 7 deletions(-)
 
 diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
 index a120f98..ae01930 100644
 --- a/kernel/trace/trace.c
 +++ b/kernel/trace/trace.c
 @@ -4194,12 +4194,6 @@ static void buffer_pipe_buf_release(struct 
 pipe_inode_info *pipe,
   buf-private = 0;
  }
  
 -static int buffer_pipe_buf_steal(struct pipe_inode_info *pipe,
 -  struct pipe_buffer *buf)
 -{
 - return 1;
 -}
 -
  static void buffer_pipe_buf_get(struct pipe_inode_info *pipe,
   struct pipe_buffer *buf)
  {
 @@ -4215,7 +4209,7 @@ static const struct pipe_buf_operations 
 buffer_pipe_buf_ops = {
   .unmap  = generic_pipe_buf_unmap,
   .confirm= generic_pipe_buf_confirm,
   .release= buffer_pipe_buf_release,
 - .steal  = buffer_pipe_buf_steal,
 + .steal  = generic_pipe_buf_steal,
   .get= buffer_pipe_buf_get,
  };
  
 

Re: [Qemu-devel] [F.A.Q.] the advantages of a shared tool/kernel Git repository, tools/perf/ and tools/kvm/

[Steven Rostedt]

On Tue, Nov 08, 2011 at 10:32:25AM +0100, Ingo Molnar wrote:
 
 None of the perf developers with whom i'm working complained about 
 the shared repo so far - publicly or privately. By all means they are 
 enjoying it and if you look at the stats and results you'll agree 
 that they are highly productive working in that environment.

Just because you brought it up.

I personally find it awkward to work in the linux tools directory. Maybe
this is the reason that I haven't been such a big contributor of perf. I
only pushed ktest into the kernel tools directory because people
convinced me to do so. Having it there didn't seem to bring in many
other developers. Only one other person has contributed to me, and that
was just some minor changes. I still find it awkward to work on ktest
inside the kernel. I have a separate tree just for ktest, and that means
I have all the kernel files sitting there doing nothing just to be able
to work on 2 files.

Then there's the issue of waiting for Linus to pull from me. I posted my
patch set on Oct 28th, and it didn't make it into the merge window. I
don't know if Linus had an issue with it, or it just got lost in the
noise, as Linus has a lot of other things to worry about. This brings up
another question. Does Linus scale? Having more tools in the kernel
repo requires Linus to pull from more sources. Or are we just going to
have to have a tools maintainer. This will give a lot of control to
that person who is the gate keeper of the tools directory.

Now I've kept trace-cmd and kernelshark outside the kernel tree. I've
received lots of patches from other developers for it and some nice new
features. It requires me to think hard to keep a nice ABI, and it has
been working nicely. The event parsing is working well and there's even
a library. But I haven't pushed it too hard because I want this to apply
to perf as well. But due to disagreements of where in the kernel tree it
belongs, it has been over a year with no progress. Now we waste 4 bytes
for every event recording a non existent big kernel lock counter. For
recording a million events (which is actually low) that's 4Megs of
wasted kernel memory. New tracepoints are going into the kernel all the
time, and without a library, we are increasing the chance that more
tools will break on changes, and tracepoints will lock down kernel
inovation soon if something is not done.

Anyway, I'm having surgery tomorrow and have other things to work on.

-- Steve