[Libguestfs] [PATCH] launch: rework handling of --enable-valgrind-daemon

[Richard W.M. Jones]

On Thu, Jun 25, 2015 at 04:16:59PM +0200, Pino Toscano wrote:
> In data giovedì 25 giugno 2015 14:50:03, Richard W.M. Jones ha scritto:
> > This doesn't handle the guestfsd shutdown case (but nor does the
> > current code, which is both racy on shutdown and introduces separate
> > shutdown paths for --enable-valgrind-daemon and ordinary
> > configurations).  But we can punt on that until later.  The above
> > would detect all memory errors except for memory leaks.
> 
> ... although losing the leaks detection would be a no-go for me, since
> that's something I've been using from time to time, even if not often.
> 
> Can you expand a bit more on the parts you consider racy?

There are two objections:

(1) That we currently have separate shutdown paths in the
valgrind/non-valgrind case:

https://github.com/libguestfs/libguestfs/blob/master/src/handle.c#L429-L443
https://github.com/libguestfs/libguestfs/blob/master/src/conn-socket.c#L346-L364

We shouldn't have two different paths that developers and non-
developers use.

(2) This actually causes bugs.  The second one is the race.

https://bugzilla.redhat.com/show_bug.cgi?id=1023630
https://bugzilla.redhat.com/show_bug.cgi?id=1020216

There's also another race which I don't think we have a bug about
where the output from valgrind is truncated (because valgrind is quite
slow at shutdown) so that you don't see valgrind errors reliably.
Attempting (not always successfully) to work around that race is the
purpose of the sleep here:

https://github.com/libguestfs/libguestfs/blob/master/appliance/init#L154

In summary, I think it's a hack.  I cannot use it because of the bugs
above, which makes it useless for me.  Finding memory leaks is
certainly important, but crashes in the daemon are way more important.
I'm not saying we couldn't fix the memory leak problem later.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/