Re: [Qemu-devel] [PATCH 1/2] trace: Add "cpu_init" event

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2333 bytes --]

On Thu, Sep 15, 2016 at 02:51:59PM +0200, Lluís Vilanova wrote:
> Stefan Hajnoczi writes:
> 
> > On Wed, Sep 14, 2016 at 06:01:17PM +0200, Lluís Vilanova wrote:
> >> Stefan Hajnoczi writes:
> >> 
> >> > On Tue, Sep 06, 2016 at 04:25:53PM +0200, Lluís Vilanova wrote:
> >> >> +## vCPU
> >> >> +
> >> >> +# Create a new virtual (guest) CPU
> >> >> +#
> >> >> +# Targets: all
> >> >> +guest_cpu_init(void *cpu) "cpu=%p"
> >> 
> >> > This isn't a vcpu trace event.  Please add keep it with the other
> >> > non-vcpu trace events:
> >> 
> >> > # cpus.c
> >> > guest_cpu_init(void *cpu) "cpu=%p"
> >> 
> >> It actually is, but as the commit message says, declaring it as such prevents
> >> the event to be emitted.
> 
> > If it cannot be toggled on a per-vcpu basis then claiming it's a vcpu
> > event doesn't make sense.
> 
> >> The culprit of this problem is that new vCPUs start with an empty per-vCPU trace
> >> event set. Should we make vCPUs "inherit" the state from the global state?
> >> (i.e., if any vcpu event is set on any vCPU, set it on the new one). The next
> >> question would then be, should this inheritance only apply until tracing is
> >> fully initialized of for the whole duration of QEMU?
> 
> > I think the underlying issue is that trace_init_vcpu_events() assumes
> > there is a single instant where vcpus all exist and need to be
> > initialized.
> 
> > A model that supports vcpu hotplug is really needed.  In that model the
> > global dstate should be the "global" state that determines whether vcpus
> > are initialized with the event enabled or disabled.
> 
> Ok, so then you're going for what I was saying. "Inheriting" the state of every
> new vCPU from the global dynamic state. If *any* vCPU has an event enabled, all
> new vCPUs will too.

No, I was thinking of something slightly different:

Right now vcpu trace events use the global dstate during intialization
only.  After trace_init_vcpu_events() their dstate is zeroed.

What if we leave the global dstate and use it as the template for
enabling events in new vCPUs (hotplugged or static)?

> As I said, next question is whether inheritance should apply only until full
> initialization, or the whole time. I thing the latter is less confusing.

I agree that it should apply the whole time.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]