Re: [PATCH 0/5] trace: inroduce qmp: trace namespace

[Stefan Hajnoczi <stefanha@redhat.com>]

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

On Thu, Oct 14, 2021 at 06:22:32PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> 12.10.2021 14:49, Markus Armbruster wrote:
> > Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:
> > 
> > > Hi all!
> > > 
> > > We have handle_qmp_command and qmp_command_repond trace points to trace
> > > qmp commands. They are very useful to debug problems involving
> > > management tools like libvirt.
> > > 
> > > But tracing all qmp commands is too much.
> > > 
> > > Here I suggest a kind of tracing namespace. Formally this series adds a
> > > trace points called qmp:<some-command> for every command, which may be
> > > enabled in separate like
> > > 
> > >    --trace qmp:drive-backup
> > > 
> > > or by pattern like
> > > 
> > >    --trace qmp:block-job-*
> > > 
> > > or similarly with help of qmp command trace-event-set-state.
> > > 
> > > This also allows to enable tracing of some qmp commands permanently
> > >   (by downstream patch or in libvirt xml). For example, I'm going to
> > > enable tracing of block job comamnds and blockdev-* commands in
> > > Virtuozzo. Qemu logs are often too empty (for example, in comparison
> > > with Libvirt), logging block jobs is not too much but will be very
> > > helpful.
> > 
> > What exactly is traced?  Peeking at PATCH 5... looks like it's input
> > that makes it to qmp_dispatch() and command responses, but not events.
> > 
> > Fine print on "input that makes it to qmp_dispatch()":
> > 
> > * You trace right before we execute the command, not when we receive,
> >    parse and enqueue input.
> > 
> > * Corollary: input with certain errors is not traced.
> > 
> > * You don't trace the input text, you trace the unparsed parse tree.
> > 
> > All fine, I presume.
> > 
> > Existing tracepoints in monitor/qmp.c, and what information they send
> > (inessential bits omitted for clarity):
> > 
> > * handle_qmp_command
> > 
> >    Handling a QMP command: unparsed parse tree
> > 
> >    Fine print, safe to ignore:
> > 
> >    - Out-of-band commands will be executed right away, in-band commands
> >      will be queued.  Tracepoints monitor_qmp_in_band_enqueue and
> >      monitor_qmp_in_band_dequeue provide insight into that.
> > 
> >    - This also receives and queues parse errors, without tracing them.
> >      Tracepoint monitor_qmp_err_in_band traces them as they are dequeued.
> > 
> > * monitor_qmp_cmd_in_band
> > 
> >    About to execute in-band command: command ID, if any
> > 
> > * monitor_qmp_cmd_out_of_band
> > 
> >    About to execute out-of-band command: command ID, if any
> > 
> > * monitor_qmp_respond
> > 
> >    About to send command response or event: QObject
> > 
> > For input, --trace qmp:* is like --trace handle_qmp_command, except it
> > traces late rather than early.
> > 
> > For output, --trace qmp:* is like --trace monitor_qmp_respond less
> > events.
> > 
> > The main improvement over existing tracepoints seems to be the ability
> > to filter on command names.
> > 
> > To get that, you overload the @name argument of QMP command
> > trace-event-set-state.  In addition to the documented meaning "Event
> > name pattern", it also has an alternate, undocumented meaning "QMP
> > command name pattern".  The "undocumented" part is easy enough to fix.
> > However, QMP heavily frowns on argument values that need to be parsed.
> 
> Still, pattern is parsed anyway, as pattern. But yes, this patch adds
> rather specific and tricky logic, which a lot more than just a pattern
> to search through the list.
> 
> Another possible way is to update QAPI code generator to insert a personal
> trace point for each qmp command.. That seems more complicated to implement,
> but I can try.

That's what came to mind when I saw this series too. The QAPI generator
can create a trace event for each QMP command. That way each command has
a dedicated trace event that can be enabled/disabled in the usual way
(e.g. built-in "trace" monitor command, SystemTap scripts, etc) without
introducing special syntax.

Stefan

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