[libvirt] [PATCH 6/7] Rewrite all the DTrace/SystemTAP probing

[Eric Blake]

On 10/07/2011 09:56 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange"<berrange at redhat.com>
>
> The libvirtd daemon had a few crude system tap probes. Some of
> these were broken during the RPC rewrite. The new modular RPC
> code is structured in a way that allows much more effective
> tracing. Instead of trying to hook up the original probes,
> define a new set of probes for the RPC and event code.
>
> The master probes file is now src/probes.d.  This contains
> probes for virNetServerClientPtr, virNetClientPtr, virSocketPtr
> virNetTLSContextPtr and virNetTLSSessionPtr modules. Also add
> probes for the poll event loop.
>
> The src/dtrace2systemtap.pl script can convert the probes.d
> file into a libvirt_probes.stp file to make use from systemtap
> much simpler.
>
> The src/rpc/gensystemtap.pl script can generate a set of
> systemtap functions for translating RPC enum values into
> printable strings. This works for all RPC header enums (program,
> type, status, procedure) and also the authentication enum
>
> The PROBE macro will automatically generate a VIR_DEBUG
> statement, so any place with a PROBE can remove any existing
> manual DEBUG statements.

Sounds useful.

>
> * daemon/libvirtd.stp, daemon/probes.d: Remove obsolete probing
> * daemon/libvirtd.h: Remove probe macros
> * daemon/Makefile.am: Remove all probe buildings/install
> * daemon/remote.c: Update authentication probes
> * src/dtrace2systemtap.pl, src/rpc/gensystemtap.pl: Scripts
>    to generate STP files
> * src/internal.h: Add probe macros
> * src/probes.d: Master list of probes
> * src/rpc/virnetclient.c, src/rpc/virnetserverclient.c,
>    src/rpc/virnetsocket.c, src/rpc/virnettlscontext.c,
>    src/util/event_poll.c: Insert probe points, removing any
>    DEBUG statements that duplicate the info
> ---
>   daemon/Makefile.am           |   26 +-----
>   daemon/libvirtd.h            |   47 ----------
>   daemon/libvirtd.stp          |   65 --------------
>   daemon/probes.d              |   12 ---
>   daemon/remote.c              |   60 ++++++++-----
>   src/.gitignore               |    4 +
>   src/Makefile.am              |   39 ++++++++-
>   src/dtrace2systemtap.pl      |  104 ++++++++++++++++++++++
>   src/internal.h               |   73 ++++++++++++++++
>   src/probes.d                 |   72 ++++++++++++++++
>   src/rpc/gensystemtap.pl      |  195 ++++++++++++++++++++++++++++++++++++++++++
>   src/rpc/virnetclient.c       |   28 +++++--
>   src/rpc/virnetserverclient.c |   23 +++++-
>   src/rpc/virnetsocket.c       |   13 +++-
>   src/rpc/virnettlscontext.c   |   47 +++++++---
>   src/util/event_poll.c        |   47 ++++++++---
>   16 files changed, 644 insertions(+), 211 deletions(-)
>   delete mode 100644 daemon/libvirtd.stp
>   delete mode 100644 daemon/probes.d
>   create mode 100644 src/dtrace2systemtap.pl
>   create mode 100644 src/probes.d
>   create mode 100644 src/rpc/gensystemtap.pl

Big, but I don't see any obvious way to split it further.

> diff --git a/src/.gitignore b/src/.gitignore
> index a619643..64e6aec 100644
> --- a/src/.gitignore
> +++ b/src/.gitignore
> @@ -16,3 +16,7 @@ libvirt_qemu.def
>   *.s
>   remote_protocol-structs-t
>   virt-aa-helper
> +libvirt_functions.stp
> +libvirt_probes.stp
> +probes.o
> +probes.h

Probably worth sorting these (or maybe someday I should go through with 
my threat of consolidating all ignores into the top-level .gitignore).

> +
> +libvirt_functions.stp: $(wildcard */*.x) $(srcdir)/rpc/gensystemtap.pl
> +	$(AM_V_GEN)perl -w $(srcdir)/rpc/gensystemtap.pl $(wildcard */*.x)>  $@

Does this still work in VPATH builds?  If not, we may need some followup 
patches.  Maybe even just $(wildcard $(srcdir)/*/*.x) is sufficient?

> +/* The double cast is necessary to silence gcc warnings; any pointer
> + * can safely go to intptr_t and back to void *, which collapses
> + * arrays into pointers; while any integer can be widened to intptr_t
> + * then cast to void *.  */
> +#  define VIR_ADD_CASTXXXX(a) ((void *)(intptr_t)(a))
> +#  define VIR_ADD_CAST(a) (a)

This doesn't quite match what used to be in libvirtd.h; in particular, 
the VIR_ADD_CASTXXXX is unused, but its body used to be in VIR_ADD_CAST. 
  What's going on there?  But this patch is already big enough that I'm 
okay if you commit it now, then we touch it up to once again fix 
compilation with systemtap 1.2 headers as a later patch.

> diff --git a/src/rpc/gensystemtap.pl b/src/rpc/gensystemtap.pl
> new file mode 100644

We probably ought to create the new .pl files as mode 100755, not 100644.

It's big, but useful, and I think that we can clean up the fallout as 
separate patches as we port it to more compilation environments (VPATH, 
older systemtap headers, etc.).  I'm okay giving ACK.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org