[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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

On 10/07/2011 09:56 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange"<berrange 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

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 redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]