[Libguestfs] [PATCH 31/67] Fix --enable-valgrind-daemon option.

[Richard W.M. Jones]

From: "Richard W.M. Jones" <rjones at redhat.com>

Don't add the "valgrind channel" to the appliance.

Just dump out the valgrind.log to stderr while the daemon is running.

Ensure that if valgrind tests fail in the appliance, that we don't
exit with success in the library by checking for a canary message in
the verbose daemon logs.

This allows the option to be used routinely by developers.

(cherry picked from commit 55e3b8711f340a2f8bdb8ee8ff99deb40b4e9108)
---
 appliance/init      | 12 ++++--------
 configure.ac        |  1 -
 src/conn-socket.c   | 14 ++++++++++++++
 src/launch-direct.c | 10 ----------
 src/launch.c        |  3 +++
 5 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/appliance/init b/appliance/init
index 681e59b..79083a4 100755
--- a/appliance/init
+++ b/appliance/init
@@ -135,21 +135,17 @@ fi
 
 if ! grep -sq guestfs_rescue=1 /proc/cmdline; then
   # Run the daemon under valgrind if ./configure --enable-valgrind-daemon
-  vg_channel=/dev/virtio-ports/org.libguestfs.valgrind
-  if [ -w $vg_channel ]; then
+  if grep -sq guestfs_valgrind_daemon=1 /proc/cmdline; then
     if [ -r /etc/guestfsd.suppressions ]; then
       suppressions="--suppressions=/etc/guestfsd.suppressions"
     fi
-    exec 3>$vg_channel
-    vg="valgrind --leak-check=full --log-fd=3 --error-exitcode=119 --max-stackframe=8388608 --child-silent-after-fork=yes $suppressions"
+    vg="valgrind --leak-check=full --error-exitcode=119 --max-stackframe=8388608 --child-silent-after-fork=yes $suppressions"
     echo "enabling valgrind: $vg"
   fi
 
-  # The host will kill qemu abruptly if guestfsd shuts down normally
+  # Run guestfsd, under valgrind if asked.
   $vg guestfsd
-
-  # Otherwise we try to clean up gracefully. For example, this ensures that a
-  # core dump generated by the guest daemon will be written to disk.
+  if [ $? -eq 119 ]; then echo "DAEMON VALGRIND FAILED"; fi
 else
   # Use appliance in rescue mode, also used by the virt-rescue command.
   eval $(grep -Eo 'TERM=[^[:space:]]+' /proc/cmdline)
diff --git a/configure.ac b/configure.ac
index 396d9fd..61cae54 100644
--- a/configure.ac
+++ b/configure.ac
@@ -385,7 +385,6 @@ if test "x$enable_daemon" = "xyes"; then
 
     if test "x$enable_valgrind_daemon" = "xyes"; then
         AC_DEFINE([VALGRIND_DAEMON],[1],[Define to 1 to run the daemon under valgrind.])
-        AC_DEFINE_UNQUOTED([VALGRIND_LOG_PATH],["$(pwd)"],[Path to save valgrind log files.])
     fi
 
     dnl Which directory should we put the daemon in?  NOTE: This
diff --git a/src/conn-socket.c b/src/conn-socket.c
index 2b3f222..e37ac23 100644
--- a/src/conn-socket.c
+++ b/src/conn-socket.c
@@ -321,6 +321,20 @@ handle_log_message (guestfs_h *g,
   /* It's an actual log message, send it upwards. */
   guestfs___log_message_callback (g, buf, n);
 
+#ifdef VALGRIND_DAEMON
+  /* Find the canary printed by appliance/init if valgrinding of the
+   * daemon fails, and exit abruptly.  Note this is only used in
+   * developer builds, and should never be enabled in ordinary/
+   * production builds.
+   */
+  if (g->verbose) {
+    const char *valgrind_canary = "DAEMON VALGRIND FAILED";
+
+    if (memmem (buf, n, valgrind_canary, strlen (valgrind_canary)) != NULL)
+      exit (119);
+  }
+#endif
+
   return 1;
 }
 
diff --git a/src/launch-direct.c b/src/launch-direct.c
index 6414f60..8d05221 100644
--- a/src/launch-direct.c
+++ b/src/launch-direct.c
@@ -434,16 +434,6 @@ launch_direct (guestfs_h *g, const char *arg)
     add_cmdline (g, "-device");
     add_cmdline (g, "virtserialport,chardev=channel0,name=org.libguestfs.channel.0");
 
-#ifdef VALGRIND_DAEMON
-    /* Set up virtio-serial channel for valgrind messages. */
-    add_cmdline (g, "-chardev");
-    snprintf (buf, sizeof buf, "file,path=%s/valgrind.log.%d,id=valgrind",
-              VALGRIND_LOG_PATH, getpid ());
-    add_cmdline (g, buf);
-    add_cmdline (g, "-device");
-    add_cmdline (g, "virtserialport,chardev=valgrind,name=org.libguestfs.valgrind");
-#endif
-
     /* Enable user networking. */
     if (g->enable_network) {
       add_cmdline (g, "-netdev");
diff --git a/src/launch.c b/src/launch.c
index 894ca15..415edc2 100644
--- a/src/launch.c
+++ b/src/launch.c
@@ -320,6 +320,9 @@ guestfs___appliance_command_line (guestfs_h *g, const char *appliance_dev,
   ret = safe_asprintf
     (g,
      "panic=1"             /* force kernel to panic if daemon exits */
+#ifdef VALGRIND_DAEMON
+     " guestfs_valgrind_daemon=1"
+#endif
 #ifdef __i386__
      " noapic"                  /* workaround for RHBZ#857026 */
 #endif
-- 
1.8.3.1