Re: [PATCH v7 08/10] tools/misc: Add xen-vmtrace tool

[Ian Jackson <iwj@xxxxxxxxxxxxxx>]

Andrew Cooper writes ("Re: [PATCH v7 08/10] tools/misc: Add xen-vmtrace tool"):
> On 22/01/2021 15:33, Ian Jackson wrote:
> > Andrew Cooper writes ("[PATCH v7 08/10] tools/misc: Add xen-vmtrace tool"):
> >> +    if ( xc_vmtrace_disable(xch, domid, vcpu) )
> >> +        perror("xc_vmtrace_disable()");
> > I guess the tracing will remain on, pointlessly, which has a perf
> > impact but nothing else ?
> 
> The perf hit is substantial, but it is safe to leave enabled.

And how does one clean up after an unclean exit of this program ?

> > How is it possible for the user to clean this up ?
> 
> For now, enable/disable can only fail with -EINVAL for calls made in the
> wrong context, so a failure here is benign in practice.
> 
> I specifically didn't opt for reference counting the enable/disable
> calls, because there cannot (usefully) be two users of this interface.

Right.

> > Also: at the very least, you need to trap SIGTERM SIGHUP SIGPIPE.
> >
> > It would be good to exit with the right signal by re-raising it.
> 
> This is example code, not a production utility.

Perhaps the utility could print some kind of health warning about it
possibly leaving this perf-impacting feature enabled, and how to clean
up ?

Ian.