Re: [libvirt] [PATCH 0/3] Experimental support for DTrace probes

On Tue, Sep 14, 2010 at 07:30:20PM +0100, Daniel P. Berrange wrote: > This patchset provides the infrastructure for supporting dynamic > probing of libvirtd, using static DTrace markers. This can be > used by SystemTAP on Linux, or DTrace on Solaris/OS-X/BSD for > low overhead tracing. > > The proof of concept provides a handful of markers wrt to network > client connections, security & auth. Obviously it can be expanded > to cover a huge area of our codebase for different tasks. The > hard bit is deciding what should be exposed as a probe point. > Ideally probes should not be changed/removed once added, since > this would break any user tracing scripts. So a little care needs > to be taken in placing probes to be robust against future code > re-factoring. Very interesting, I'm just a bit surprized by the patch set. patch 1 and 2 are really unrelated, I think they should go in independantly. Patch 1 is pure refactoring, I would rather apply it early in the cycle for 0.8.5 (or whatever our next release might be named), and glancing at it it looks finr to me ACK (will jsut need a bit of rebase due to Justin patch) Patch 2 looks simple enough, ACK too For patch 3 I'm a bit surprized, I think I would have started by adding probes for all the public API places first on entry and second on exit with the return value provided. It can be helpful for debugging connection problems but well I would use SystemTap more for debugging and tuning of a running system (i.e. it runs and you don't want to disturb it or minimally)

We should understand the problem of the restart being needed to before pushing patch3 I think, maybe we need to talk to one of the SystemTap developpers so he can explain what is going on and how to fix this :-)