Re: [PULL 10/11] trace: document how to specify multiple --trace patterns

[BALATON Zoltan]

On Mon, 1 Feb 2021, Daniel P. Berrangé wrote:
> On Mon, Feb 01, 2021 at 06:25:33PM +0100, Paolo Bonzini wrote:
> > On 01/02/21 17:54, Kevin Wolf wrote:
> > > > How does this option parsing work? Would then multiple patterns separated by
> > > > comma as in -trace pattern1,pattern2 also work?
> > > This would be interpreted as an implied "enable" option with a value of
> > > "pattern1,pattern2". I don't think anything splits that string at the
> > > comma, so it would look for a trace event matching that string.
> >
> > Even worse, it would be interpreted as "-trace enable=pattern1,pattern2=on"
> > (and raise a warning since recently).
>
> Maybe we're trying to solve the problem at the wrong level.

There's no problem to solve, just trying to understand better what are the 
valid options. It's already possible to enable multiple patterns with 
either events=file or repeating -trace options (with or without enable=) 
so that's already sufficient, I was just curious what other options are 
there and if there's a simpler way that we could document. If not, using 
the current ways that are now documented is OK I think.

> The pattern is currently matched using the GLib glob matching APIs.
>
> If we switched to use the GLib regex matching APIs, then we don't need
> to repeat the args at all. We could just use regex syntax:
>
>  -trace 'enable=(kvm|virtio)*'
>
> It is a little tedious to have to quote the args to avoid shell
> expansion, but as a tradeoff you get much stronger ability todo
> complex matches to filter out irrelevant cruft.

I guess we have enough expressiveness with current pattern matching and 
globs are more easily understood by mere users so regex may not be needed 
here.

> If we want to maintain back compat for glob syntax, then we should
> support both in parallel and accept a different parameter name
> for the regex style.

That would be (even more) confusing so better to just stay with globs.

Regards,
BALATON Zoltan