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

On Thu, Jun 25, 2015 at 04:16:59PM +0200, Pino Toscano wrote: 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#L3... 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/