Libguestfs - Sep 2015 - [PATCH] [repost] build: Remove ./configure --enable-valgrind-daemon.

Previously posted here:
https://www.redhat.com/archives/libguestfs/2015-June/msg00266.html

Rich.

If you've ever tried to use this option, you'll know that it didn't
work well.  It broke random things (probably RHBZ#1020216, definitely
RHBZ#1023630), and caused random failures generally, while often not
actually failing when valgrind itself found problems.
---
 appliance/Makefile.am           |  7 +------
 appliance/guestfsd.suppressions | 27 ---------------------------
 appliance/init                  | 21 +--------------------
 appliance/packagelist.in        |  2 --
 configure.ac                    | 14 --------------
 generator/actions.ml            |  3 +--
 src/conn-socket.c               | 20 --------------------
 src/guestfs-internal.h          | 12 ------------
 src/handle.c                    | 15 ---------------
 src/launch-uml.c                | 11 -----------
 src/launch.c                    |  3 ---
 11 files changed, 3 insertions(+), 132 deletions(-)
 delete mode 100644 appliance/guestfsd.suppressions

diff --git a/appliance/Makefile.am b/appliance/Makefile.am
index d21e743..d47fd23 100644
--- a/appliance/Makefile.am
+++ b/appliance/Makefile.am
@@ -20,7 +20,6 @@ include $(top_srcdir)/subdir-rules.mk
 EXTRA_DIST = \
 	99-guestfs-serial.rules \
 	excludefiles.in \
-	guestfsd.suppressions \
 	guestfs_lvm_conf.aug \
 	guestfs_shadow.aug \
 	hostfiles.in \
@@ -66,9 +65,6 @@ make.sh: make.sh.in $(top_builddir)/config.log
$(top_builddir)/config.status
 	rm -f $@-t
 
 PACKAGELIST_CPP_FLAGS = -D$(DISTRO)=1
-DEXTRA_PACKAGES="$(EXTRA_PACKAGES)"
-if VALGRIND_DAEMON
-PACKAGELIST_CPP_FLAGS += -DVALGRIND_DAEMON=1
-endif
 
 packagelist: packagelist.in Makefile
 	m4 $(PACKAGELIST_CPP_FLAGS) $< | \
@@ -76,12 +72,11 @@ packagelist: packagelist.in Makefile
 	cmp -s $@ $@-t || mv $@-t $@
 	rm -f $@-t
 
-supermin.d/daemon.tar.gz: ../daemon/guestfsd guestfsd.suppressions
guestfs_lvm_conf.aug guestfs_shadow.aug
+supermin.d/daemon.tar.gz: ../daemon/guestfsd guestfs_lvm_conf.aug
guestfs_shadow.aug
 	rm -f $@ $@-t
 	rm -rf tmp-d
 	mkdir -p tmp-d$(DAEMON_SUPERMIN_DIR) tmp-d/etc tmp-d/usr/share/guestfs
 	ln ../daemon/guestfsd tmp-d$(DAEMON_SUPERMIN_DIR)/guestfsd
-	ln $(srcdir)/guestfsd.suppressions tmp-d/etc/guestfsd.suppressions
 	ln $(srcdir)/guestfs_lvm_conf.aug tmp-d/usr/share/guestfs/guestfs_lvm_conf.aug
 	ln $(srcdir)/guestfs_shadow.aug tmp-d/usr/share/guestfs/guestfs_shadow.aug
 	( cd tmp-d && tar zcf - * ) > $@-t
diff --git a/appliance/guestfsd.suppressions b/appliance/guestfsd.suppressions
deleted file mode 100644
index 26ba1c8..0000000
--- a/appliance/guestfsd.suppressions
+++ /dev/null
@@ -1,27 +0,0 @@
-# This file is only used when libguestfs is configured with
-#
-#   ./configure --enable-valgrind-daemon
-#
-# (only used for development, and only used in the regular supermin
-# appliance, not libguestfs live).
-#
-# If there are any valgrind errors in the base libraries such as
-# glibc, then we can suppress them here, so we only see errors in
-# libguestfs daemon code.
-
-# libdl
-{
-  libdl_index_cond
-  Memcheck:Cond
-  fun:index
-  fun:expand_dynamic_string_token
-  fun:_dl_map_object
-}
-
-# aug_setm memory leak
-{
-  aug_setm_leak
-  Memcheck:Leak
-  ...
-  fun:aug_setm
-}
diff --git a/appliance/init b/appliance/init
index d5a428d..ee8c60e 100755
--- a/appliance/init
+++ b/appliance/init
@@ -150,18 +150,7 @@ fi
 
 if ! test "$guestfs_rescue" = 1; then
   # Run the daemon.
-
-  # Run the daemon under valgrind if ./configure --enable-valgrind-daemon
-  if grep -sq guestfs_valgrind_daemon=1 /proc/cmdline; then
-    if [ -r /etc/guestfsd.suppressions ]; then
-      suppressions="--suppressions=/etc/guestfsd.suppressions"
-    fi
-    vg="valgrind --leak-check=full --error-exitcode=119
--max-stackframe=8388608 --child-silent-after-fork=yes $suppressions"
-    echo "enabling valgrind: $vg"
-  fi
-
-  # Run guestfsd, under valgrind if asked.
-  cmd="$vg guestfsd"
+  cmd="guestfsd"
   if test "x$guestfs_channel" != "x"; then
     cmd="$cmd --channel $guestfs_channel"
   fi
@@ -173,14 +162,6 @@ if ! test "$guestfs_rescue" = 1; then
   fi
   echo $cmd
   $cmd
-
-  if [ $? -eq 119 ]; then
-      echo "DAEMON VALGRIND FAILED"
-      # Sleep so valgrind messages are seen by the host.  Note this
-      # only happens in non-production builds
-      # (--enable-valgrind-daemon) + on an error path.
-      sleep 10
-  fi
 else
   # Run virt-rescue shell.
 
diff --git a/appliance/packagelist.in b/appliance/packagelist.in
index 5896330..fee6fa0 100644
--- a/appliance/packagelist.in
+++ b/appliance/packagelist.in
@@ -283,7 +283,5 @@ curl
 dnl (virt-dib) tools optionally used for elements
 debootstrap
 
-ifelse(VALGRIND_DAEMON,1,valgrind)
-
 dnl Define this by doing: ./configure --with-extra-packages="..."
 EXTRA_PACKAGES
diff --git a/configure.ac b/configure.ac
index 86ed4a4..fad7228 100644
--- a/configure.ac
+++ b/configure.ac
@@ -435,19 +435,6 @@ if test "x$enable_daemon" = "xyes";
then
             [enable_install_daemon=no])
     AC_MSG_RESULT([$enable_install_daemon])
 
-    dnl Enable valgrind in the daemon.
-    AC_MSG_CHECKING([if we should run the daemon under valgrind])
-    AC_ARG_ENABLE([valgrind-daemon],
-        [AS_HELP_STRING([--enable-valgrind-daemon],
-            [run the daemon under valgrind (developers only)
@<:@default=no@:>@])],
-            [],
-            [enable_valgrind_daemon=no])
-    AC_MSG_RESULT([$enable_valgrind_daemon])
-
-    if test "x$enable_valgrind_daemon" = "xyes"; then
-        AC_DEFINE([VALGRIND_DAEMON],[1],[Define to 1 to run the daemon under
valgrind.])
-    fi
-
     dnl Which directory should we put the daemon in?  NOTE: This
     dnl is the "virtual" directory inside the appliance, not the
     dnl install directory for libguestfs live.  Since Fedora 17
@@ -487,7 +474,6 @@ This means you either have a very old glibc (pre-2.0) or you
 are using some other libc where this is not supported.])])])
 fi
 AM_CONDITIONAL([INSTALL_DAEMON],[test "x$enable_install_daemon" =
"xyes"])
-AM_CONDITIONAL([VALGRIND_DAEMON],[test "x$enable_valgrind_daemon" =
"xyes"])
 
 dnl Build the appliance?
 AC_MSG_CHECKING([if we should build the appliance])
diff --git a/generator/actions.ml b/generator/actions.ml
index 2f0ad0b..12d5e5a 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -12023,8 +12023,7 @@ This function is used internally when setting up the
appliance." };
     cancellable = true;
     shortdesc = "cause the daemon to exit (internal use only)";
     longdesc = "\
-This function is used internally when closing the appliance.  Note
-it's only called when ./configure --enable-valgrind-daemon is used."
};
+This function is used internally when testing the appliance." };
 
   { defaults with
     name = "copy_attributes"; added = (1, 25, 21);
diff --git a/src/conn-socket.c b/src/conn-socket.c
index 3ead48f..980d7df 100644
--- a/src/conn-socket.c
+++ b/src/conn-socket.c
@@ -343,26 +343,6 @@ handle_log_message (guestfs_h *g,
   /* It's an actual log message, send it upwards. */
   guestfs_int_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) {
-      fprintf (stderr,
-               "Detected valgrind failure in the daemon!  Exiting with
exit code 119.\n"
-               "See log messages printed above.\n"
-               "Note: This happens because libguestfs was configured
with\n"
-               "'--enable-valgrind-daemon' which should not be
used in production builds.\n");
-      exit (119);
-    }
-  }
-#endif
-
   return 1;
 }
 
diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
index 9c7175f..247e8b2 100644
--- a/src/guestfs-internal.h
+++ b/src/guestfs-internal.h
@@ -78,18 +78,6 @@
 #  define MIN_MEMSIZE 256
 #endif
 
-/* Valgrind has a fairly hefty memory overhead.  Using the defaults
- * caused the C API tests to fail.
- */
-#ifdef VALGRIND_DAEMON
-#  ifndef DEFAULT_MEMSIZE
-#    define DEFAULT_MEMSIZE 768
-#  endif
-#  ifndef MIN_MEMSIZE
-#    define MIN_MEMSIZE 256
-#  endif
-#endif
-
 /* The default and minimum memory size for most users. */
 #ifndef DEFAULT_MEMSIZE
 #  define DEFAULT_MEMSIZE 500
diff --git a/src/handle.c b/src/handle.c
index 1bfb9ae..24d3257 100644
--- a/src/handle.c
+++ b/src/handle.c
@@ -426,21 +426,6 @@ shutdown_backend (guestfs_h *g, int check_for_errors)
   if (g->autosync && g->state == READY) {
     if (guestfs_internal_autosync (g) == -1)
       ret = -1;
-#ifdef VALGRIND_DAEMON
-    /* When valgrinding the daemon, this causes the daemon to exit
-     * properly, so we'll see any valgrind problems.  Don't do this in
-     * production builds because it's slow and unnecessary.
-     */
-    guestfs_internal_exit (g);
-    if (g->conn) {
-      /* internal_exit above will cause the daemon to close the
-       * connection.  The purpose of the read_data here is to read the
-       * remaining log messages.
-       */
-      char buf[1];
-      g->conn->ops->read_data (g, g->conn, buf, sizeof buf);
-    }
-#endif
   }
 
   /* Shut down the backend. */
diff --git a/src/launch-uml.c b/src/launch-uml.c
index 785548c..6a63b6b 100644
--- a/src/launch-uml.c
+++ b/src/launch-uml.c
@@ -267,17 +267,6 @@ launch_uml (guestfs_h *g, void *datav, const char *arg)
   ADD_CMDLINE_PRINTF ("ssl3=fd:%d", dsv[1]);
   ADD_CMDLINE ("guestfs_channel=/dev/ttyS3");
 
-#if 0 /* XXX This could be made to work. */
-#ifdef VALGRIND_DAEMON
-  /* Set up virtio-serial channel for valgrind messages. */
-  ADD_CMDLINE ("-chardev");
-  ADD_CMDLINE_PRINTF ("file,path=%s/valgrind.log.%d,id=valgrind",
-                      VALGRIND_LOG_PATH, getpid ());
-  ADD_CMDLINE ("-device");
-  ADD_CMDLINE
("virtserialport,chardev=valgrind,name=org.libguestfs.valgrind");
-#endif
-#endif
-
   /* Add any vmlinux parameters. */
   for (hp = g->hv_params; hp; hp = hp->next) {
     ADD_CMDLINE (hp->hv_param);
diff --git a/src/launch.c b/src/launch.c
index 343f4ea..6bd82ab 100644
--- a/src/launch.c
+++ b/src/launch.c
@@ -335,9 +335,6 @@ guestfs_int_appliance_command_line (guestfs_h *g, const char
*appliance_dev,
 #ifdef __arm__
      " mem=%dM"
 #endif
-#ifdef VALGRIND_DAEMON
-     " guestfs_valgrind_daemon=1"
-#endif
 #ifdef __i386__
      " noapic"                  /* workaround for RHBZ#857026 */
 #endif
-- 
2.5.0