[Libguestfs] [PATCH v2 1/9] build: Remove ./configure --enable-valgrind-daemon.
Richard W.M. Jones
rjones at redhat.com
Thu Jun 25 14:56:53 UTC 2015
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.
This does not remove the guestfs_internal_exit API. That will
instead be modified in a future commit.
---
appliance/Makefile.am | 7 +------
appliance/guestfsd.suppressions | 27 ---------------------------
appliance/init | 20 +-------------------
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(+), 131 deletions(-)
delete mode 100644 appliance/guestfsd.suppressions
diff --git a/appliance/Makefile.am b/appliance/Makefile.am
index 3135219..cc4bfc2 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 eb1b487..3a185c8 100755
--- a/appliance/init
+++ b/appliance/init
@@ -134,25 +134,7 @@ fi
if ! grep -sq guestfs_rescue=1 /proc/cmdline; 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.
- $vg guestfsd
- 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
+ guestfsd
else
# Run virt-rescue shell.
diff --git a/appliance/packagelist.in b/appliance/packagelist.in
index 76c7293..0f0ae89 100644
--- a/appliance/packagelist.in
+++ b/appliance/packagelist.in
@@ -253,7 +253,5 @@ util-linux-ng
xfsprogs
zerofree
-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 bee8c94..32fedd3 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 11b8652..24e84b5 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -11955,8 +11955,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 bbd7fb4..cf6e534 100644
--- a/src/guestfs-internal.h
+++ b/src/guestfs-internal.h
@@ -79,18 +79,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 51b9572..b15a15d 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 fd5479e..987d2db 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.3.1
More information about the Libguestfs
mailing list