Libguestfs - Jun 2015 - [PATCH v2 1/9] build: Remove ./configure --enable-valgrind-daemon.

In v2:

 - Kernel command line parsing now moved to the appliance.

 - In the captive daemon test, the daemon cleanly shuts down on exit.

 - Add another btrfs test.

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.

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

We'll now just require that valgrind is new enough.

As best as I can tell from the valgrind subversion(!) repository, this
option was added to valgrind in around 2011.
---
 configure.ac | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/configure.ac b/configure.ac
index 32fedd3..8775c4b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1069,21 +1069,10 @@ AS_IF([$CXX --version >&AS_MESSAGE_LOG_FD
2>&1],[have_cxx=yes],[have_cxx=no])
 AC_MSG_RESULT([$have_cxx])
 AM_CONDITIONAL([HAVE_CXX], [test "$have_cxx" = "yes"])
 
-dnl If valgrind is present (it's not required), check whether or not
-dnl it supports the new 'valgrind --vgdb' option.
 AC_CHECK_PROG([VALGRIND],[valgrind],[valgrind],[no])
 AS_IF([test "x$VALGRIND" != "xno"],[
-    AC_MSG_CHECKING([if $VALGRIND supports the --vgdb option])
-    if $VALGRIND --help | grep -sq -- --vgdb; then
-        AC_MSG_RESULT([yes])
-        AC_SUBST([VALGRIND_NO_VGDB],[--vgdb=no])
-    else
-        AC_MSG_RESULT([no])
-        AC_SUBST([VALGRIND_NO_VGDB],[])
-    fi
-
     # Substitute the whole valgrind command.
-    VG='$(VALGRIND) $(VALGRIND_NO_VGDB)
--log-file=$(abs_top_builddir)/tmp/valgrind-%q{T}-%p.log --leak-check=full
--error-exitcode=119 --suppressions=$(abs_top_srcdir)/valgrind-suppressions'
+    VG='$(VALGRIND) --vgdb=no
--log-file=$(abs_top_builddir)/tmp/valgrind-%q{T}-%p.log --leak-check=full
--error-exitcode=119 --suppressions=$(abs_top_srcdir)/valgrind-suppressions'
     ],[
     # No valgrind, so substitute VG with something that will break.
     VG=VALGRIND_IS_NOT_INSTALLED
-- 
2.3.1

This is a catch-all mode for turning off daemon features which
interfere with our testing of the daemon.
---
 daemon/daemon.h   |  2 ++
 daemon/guestfsd.c | 17 ++++++++++++++---
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/daemon/daemon.h b/daemon/daemon.h
index 136e9a9..e977095 100644
--- a/daemon/daemon.h
+++ b/daemon/daemon.h
@@ -47,6 +47,8 @@ extern int enable_network;
 
 extern int autosync_umount;
 
+extern int test_mode;
+
 extern const char *sysroot;
 extern size_t sysroot_len;
 
diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c
index eef24d5..8285d27 100644
--- a/daemon/guestfsd.c
+++ b/daemon/guestfsd.c
@@ -120,6 +120,9 @@ size_t sysroot_len = 8;
 /* If set (the default), do 'umount-all' when performing autosync. */
 int autosync_umount = 1;
 
+/* If set, we are testing the daemon as part of the libguestfs tests. */
+int test_mode = 0;
+
 /* Not used explicitly, but required by the gnulib 'error' module. */
 const char *program_name = "guestfsd";
 
@@ -136,9 +139,10 @@ usage (void)
 int
 main (int argc, char *argv[])
 {
-  static const char *options = "rv?";
+  static const char *options = "rtv?";
   static const struct option long_options[] = {
     { "help", 0, 0, '?' },
+    { "test", 0, 0, 't' },
     { "verbose", 0, 0, 'v' },
     { 0, 0, 0, 0 }
   };
@@ -191,6 +195,11 @@ main (int argc, char *argv[])
       autosync_umount = 0;
       break;
 
+      /* Undocumented --test option used for testing guestfsd. */
+    case 't':
+      test_mode = 1;
+      break;
+
     case 'v':
       verbose = 1;
       break;
@@ -244,7 +253,8 @@ main (int argc, char *argv[])
    * environment is essentially empty.
    * https://bugzilla.redhat.com/show_bug.cgi?id=502074#c5
    */
-  setenv ("PATH", "/sbin:/usr/sbin:/bin:/usr/bin", 1);
+  if (!test_mode)
+    setenv ("PATH", "/sbin:/usr/sbin:/bin:/usr/bin", 1);
   setenv ("SHELL", "/bin/sh", 1);
   setenv ("LC_ALL", "C", 1);
   setenv ("TERM", "dumb", 1);
@@ -263,7 +273,8 @@ main (int argc, char *argv[])
   /* Make a private copy of /etc/lvm so we can change the config (see
    * daemon/lvm-filter.c).
    */
-  copy_lvm ();
+  if (!test_mode)
+    copy_lvm ();
 
   /* Connect to virtio-serial channel. */
   char *channel, *p;
-- 
2.3.1

This option, used for testing, causes the daemon to create the Unix
domain socket (from guestfs_channel), listen on it, and accept a
single connection.
---
 daemon/guestfsd.c   | 65 ++++++++++++++++++++++++++++++++++++++---------------
 daemon/guestfsd.pod |  9 ++++++++
 2 files changed, 56 insertions(+), 18 deletions(-)

diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c
index 8285d27..1bcdfa3 100644
--- a/daemon/guestfsd.c
+++ b/daemon/guestfsd.c
@@ -139,15 +139,17 @@ usage (void)
 int
 main (int argc, char *argv[])
 {
-  static const char *options = "rtv?";
+  static const char *options = "lrtv?";
   static const struct option long_options[] = {
     { "help", 0, 0, '?' },
+    { "listen", 0, 0, 'l' },
     { "test", 0, 0, 't' },
     { "verbose", 0, 0, 'v' },
     { 0, 0, 0, 0 }
   };
   int c;
   char *cmdline;
+  int listen_mode = 0;
 
   ignore_value (chdir ("/"));
 
@@ -186,6 +188,10 @@ main (int argc, char *argv[])
     if (c == -1) break;
 
     switch (c) {
+    case 'l':
+      listen_mode = 1;
+      break;
+
       /* The -r flag is used when running standalone.  It changes
        * several aspects of the daemon.
        */
@@ -292,23 +298,46 @@ main (int argc, char *argv[])
   if (verbose)
     printf ("trying to open virtio-serial channel '%s'\n",
channel);
 
-  int sock = open (channel, O_RDWR|O_CLOEXEC);
-  if (sock == -1) {
-    fprintf (stderr,
-             "\n"
-             "Failed to connect to virtio-serial channel.\n"
-             "\n"
-             "This is a fatal error and the appliance will now
exit.\n"
-             "\n"
-             "Usually this error is caused by either QEMU or the
appliance\n"
-             "kernel not supporting the vmchannel method that the\n"
-             "libguestfs library chose to use.  Please run\n"
-             "'libguestfs-test-tool' and provide the complete,
unedited\n"
-             "output to the libguestfs developers, either in a bug
report\n"
-             "or on the libguestfs redhat com mailing list.\n"
-             "\n");
-    perror (channel);
-    exit (EXIT_FAILURE);
+  int sock;
+  if (!listen_mode) {
+    sock = open (channel, O_RDWR|O_CLOEXEC);
+    if (sock == -1) {
+      fprintf (stderr,
+               "\n"
+               "Failed to connect to virtio-serial channel.\n"
+               "\n"
+               "This is a fatal error and the appliance will now
exit.\n"
+               "\n"
+               "Usually this error is caused by either QEMU or the
appliance\n"
+               "kernel not supporting the vmchannel method that
the\n"
+               "libguestfs library chose to use.  Please run\n"
+               "'libguestfs-test-tool' and provide the complete,
unedited\n"
+               "output to the libguestfs developers, either in a bug
report\n"
+               "or on the libguestfs redhat com mailing list.\n"
+               "\n");
+      perror (channel);
+      exit (EXIT_FAILURE);
+    }
+  }
+  else {
+    struct sockaddr_un addr;
+
+    sock = socket (AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0);
+    if (sock == -1)
+      error (EXIT_FAILURE, errno, "socket");
+    addr.sun_family = AF_UNIX;
+    strncpy (addr.sun_path, channel, UNIX_PATH_MAX);
+    addr.sun_path[UNIX_PATH_MAX-1] = '\0';
+
+    if (bind (sock, (struct sockaddr *) &addr, sizeof addr) == -1)
+      error (EXIT_FAILURE, errno, "bind: %s", channel);
+
+    if (listen (sock, 4) == -1)
+      error (EXIT_FAILURE, errno, "listen");
+
+    sock = accept4 (sock, NULL, NULL, SOCK_CLOEXEC);
+    if (sock == -1)
+      error (EXIT_FAILURE, errno, "accept");
   }
 
   /* If it's a serial-port like device then it probably has echoing
diff --git a/daemon/guestfsd.pod b/daemon/guestfsd.pod
index 1ed31a9..22c8003 100644
--- a/daemon/guestfsd.pod
+++ b/daemon/guestfsd.pod
@@ -50,6 +50,15 @@ removed.
 
 Display brief help.
 
+=item B<-l>
+
+=item B<--listen>
+
+Instead of opening the C<guestfs_channel> and thus expecting that it
+already exists, create the channel as a Unix domain socket, listen on
+it, and accept a single connection.  This is mainly used for testing
+the daemon.
+
 =item B<-r>
 
 Set the root filesystem to be F</> (instead of the default which is
-- 
2.3.1

Instead of parsing /proc/cmdline from the daemon, move all of that
parsing into the init script, and pass the argument via the daemon
command line.

For example, previously the daemon and init script both looked for
guestfs_network=1 in /proc/cmdline.  Now the init script still looks
for it, and if found it runs `guestfsd --network'.
---
 appliance/init      | 36 ++++++++++++++++---
 daemon/guestfsd.c   | 99 ++++++++---------------------------------------------
 daemon/guestfsd.pod | 14 ++++++++
 3 files changed, 59 insertions(+), 90 deletions(-)

diff --git a/appliance/init b/appliance/init
index 3a185c8..dca3b40 100755
--- a/appliance/init
+++ b/appliance/init
@@ -88,11 +88,26 @@ for f in /sys/block/{h,s,ub,v}d*/queue/scheduler; do echo
noop > $f; done
 # Update the system clock.
 hwclock -u -s
 
+# Parse the kernel command line.
+if grep -sq guestfs_verbose=1 /proc/cmdline; then
+    guestfs_verbose=1
+fi
+if grep -sq guestfs_network=1 /proc/cmdline; then
+    guestfs_network=1
+fi
+if grep -sq guestfs_rescue=1 /proc/cmdline; then
+    guestfs_rescue=1
+fi
+if grep -sq guestfs_noreboot=1 /proc/cmdline; then
+    guestfs_noreboot=1
+fi
+eval `grep -Eo 'guestfs_channel=[^[:space:]]+' /proc/cmdline`
+
 # Set up the network.
 ip addr add 127.0.0.1/8 brd + dev lo scope host
 ip link set dev lo up
 
-if grep -sq guestfs_network=1 /proc/cmdline; then
+if test "$guestfs_network" = 1; then
     if dhclient --version >/dev/null 2>&1; then
         dhclient
     else
@@ -113,7 +128,7 @@ lvm vgchange -aay --sysinit
 ldmtool create all
 
 # These are useful when debugging.
-if grep -sq guestfs_verbose=1 /proc/cmdline; then
+if test "$guestfs_verbose" = 1; then
     uname -a
     ls -lR /dev
     cat /proc/mounts
@@ -132,9 +147,20 @@ if grep -sq guestfs_verbose=1 /proc/cmdline; then
     echo -n "uptime: "; cat /proc/uptime
 fi
 
-if ! grep -sq guestfs_rescue=1 /proc/cmdline; then
+if ! test "$guestfs_rescue" = 1; then
   # Run the daemon.
-  guestfsd
+  cmd="guestfsd"
+  if test "x$guestfs_channel" != "x"; then
+    cmd="$cmd --channel $guestfs_channel"
+  fi
+  if test "$guestfs_verbose" = 1; then
+    cmd="$cmd --verbose"
+  fi
+  if test "$guestfs_network" = 1; then
+    cmd="$cmd --network"
+  fi
+  echo $cmd
+  $cmd
 else
   # Run virt-rescue shell.
 
@@ -163,7 +189,7 @@ fi
 
 sync
 
-if ! grep -sq guestfs_noreboot=1 /proc/cmdline; then
+if ! test "$guestfs_noreboot" = 1; then
   # qemu has the -no-reboot flag, so issuing a reboot here actually
   # causes qemu to exit gracefully.
   reboot -f
diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c
index 1bcdfa3..33ada59 100644
--- a/daemon/guestfsd.c
+++ b/daemon/guestfsd.c
@@ -58,8 +58,6 @@
 
 GUESTFSD_EXT_CMD(str_udevadm, udevadm);
 
-static char *read_cmdline (void);
-
 #ifndef MAX
 # define MAX(a,b) ((a)>(b)?(a):(b))
 #endif
@@ -139,16 +137,18 @@ usage (void)
 int
 main (int argc, char *argv[])
 {
-  static const char *options = "lrtv?";
+  static const char *options = "c:lnrtv?";
   static const struct option long_options[] = {
     { "help", 0, 0, '?' },
+    { "channel", 1, 0, 'c' },
     { "listen", 0, 0, 'l' },
+    { "network", 0, 0, 'n' },
     { "test", 0, 0, 't' },
     { "verbose", 0, 0, 'v' },
     { 0, 0, 0, 0 }
   };
   int c;
-  char *cmdline;
+  const char *channel = NULL;
   int listen_mode = 0;
 
   ignore_value (chdir ("/"));
@@ -188,10 +188,18 @@ main (int argc, char *argv[])
     if (c == -1) break;
 
     switch (c) {
+    case 'c':
+      channel = optarg;
+      break;
+
     case 'l':
       listen_mode = 1;
       break;
 
+    case 'n':
+      enable_network = 1;
+      break;
+
       /* The -r flag is used when running standalone.  It changes
        * several aspects of the daemon.
        */
@@ -225,23 +233,6 @@ main (int argc, char *argv[])
     exit (EXIT_FAILURE);
   }
 
-  cmdline = read_cmdline ();
-
-  /* Set the verbose flag. */
-  verbose = verbose ||
-    (cmdline && strstr (cmdline, "guestfs_verbose=1") !=
NULL);
-  if (verbose)
-    printf ("verbose daemon enabled\n");
-
-  if (verbose) {
-    if (cmdline)
-      printf ("linux command line: %s\n", cmdline);
-    else
-      printf ("could not read linux command line\n");
-  }
-
-  enable_network = cmdline && strstr (cmdline,
"guestfs_network=1") != NULL;
-
 #ifndef WIN32
   /* Make sure SIGPIPE doesn't kill us. */
   struct sigaction sa;
@@ -283,17 +274,8 @@ main (int argc, char *argv[])
     copy_lvm ();
 
   /* Connect to virtio-serial channel. */
-  char *channel, *p;
-  if (cmdline && (p = strstr (cmdline, "guestfs_channel="))
!= NULL) {
-    p += 16;
-    channel = strndup (p, strcspn (p, " \n"));
-  }
-  else
-    channel = strdup (VIRTIO_SERIAL_CHANNEL);
-  if (!channel) {
-    perror ("strdup");
-    exit (EXIT_FAILURE);
-  }
+  if (!channel)
+    channel = VIRTIO_SERIAL_CHANNEL;
 
   if (verbose)
     printf ("trying to open virtio-serial channel '%s'\n",
channel);
@@ -346,10 +328,6 @@ main (int argc, char *argv[])
   if (STRPREFIX (channel, "/dev/ttyS"))
     makeraw (channel, sock);
 
-  /* cmdline, channel not used after this point */
-  free (cmdline);
-  free (channel);
-
   /* Wait for udev devices to be created.  If you start libguestfs,
    * especially with disks that contain complex (eg. mdadm) data
    * already, then it is possible for the 'mdadm' and LVM commands
@@ -383,55 +361,6 @@ main (int argc, char *argv[])
   exit (EXIT_SUCCESS);
 }
 
-/* Read /proc/cmdline. */
-static char *
-read_cmdline (void)
-{
-  int fd = open ("/proc/cmdline", O_RDONLY|O_CLOEXEC);
-  if (fd == -1) {
-    perror ("/proc/cmdline");
-    return NULL;
-  }
-
-  size_t len = 0;
-  ssize_t n;
-  char buf[256];
-  char *r = NULL;
-
-  for (;;) {
-    n = read (fd, buf, sizeof buf);
-    if (n == -1) {
-      perror ("read");
-      free (r);
-      close (fd);
-      return NULL;
-    }
-    if (n == 0)
-      break;
-    char *newr = realloc (r, len + n + 1); /* + 1 is for terminating NUL */
-    if (newr == NULL) {
-      perror ("realloc");
-      free (r);
-      close (fd);
-      return NULL;
-    }
-    r = newr;
-    memcpy (&r[len], buf, n);
-    len += n;
-  }
-
-  if (r)
-    r[len] = '\0';
-
-  if (close (fd) == -1) {
-    perror ("close");
-    free (r);
-    return NULL;
-  }
-
-  return r;
-}
-
 /* Try to make the socket raw, but don't fail if it's not possible. */
 static void
 makeraw (const char *channel, int fd)
diff --git a/daemon/guestfsd.pod b/daemon/guestfsd.pod
index 22c8003..f1672ae 100644
--- a/daemon/guestfsd.pod
+++ b/daemon/guestfsd.pod
@@ -50,6 +50,14 @@ removed.
 
 Display brief help.
 
+=item B<-c> CHANNEL
+
+=item B<--channel> CHANNEL
+
+Pass the name of the virtio-serial channel, serial port, etc.  over
+which guestfsd will communicate with the library.  If this parameter
+is not given, then an internal default port is used.
+
 =item B<-l>
 
 =item B<--listen>
@@ -59,6 +67,12 @@ already exists, create the channel as a Unix domain socket,
listen on
 it, and accept a single connection.  This is mainly used for testing
 the daemon.
 
+=item B<-n>
+
+=item B<--network>
+
+Enable network features in the daemon.
+
 =item B<-r>
 
 Set the root filesystem to be F</> (instead of the default which is
-- 
2.3.1

This allows us to test the daemon running as a host process, allowing
us to meaningfully test it using valgrind.

This commit only adds a single test that check that the daemon starts
up, can be pinged, and exits.
---
 .gitignore                        |   1 +
 Makefile.am                       |   1 +
 configure.ac                      |   2 +
 tests/daemon/Makefile.am          |  36 +++++++++++
 tests/daemon/captive-daemon.pm.in | 125 ++++++++++++++++++++++++++++++++++++++
 tests/daemon/test-daemon-start.pl |  35 +++++++++++
 6 files changed, 200 insertions(+)
 create mode 100644 tests/daemon/Makefile.am
 create mode 100644 tests/daemon/captive-daemon.pm.in
 create mode 100755 tests/daemon/test-daemon-start.pl

diff --git a/.gitignore b/.gitignore
index 6f14915..6089122 100644
--- a/.gitignore
+++ b/.gitignore
@@ -528,6 +528,7 @@ Makefile.in
 /tests/data/lib-i586.so.xz
 /tests/data/test-grep.txt.gz
 /tests/data/test.iso
+/tests/daemon/captive-daemon.pm
 /tests/disks/test-qemu-drive-libvirt.xml
 /tests/events/test-libvirt-auth-callbacks
 /tests/guests/blank-*.img
diff --git a/Makefile.am b/Makefile.am
index ad6d9d3..c545ea1 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -30,6 +30,7 @@ SUBDIRS += tests/data generator src examples po
 
 if ENABLE_DAEMON
 SUBDIRS += daemon
+SUBDIRS += tests/daemon
 endif
 if ENABLE_APPLIANCE
 SUBDIRS += appliance
diff --git a/configure.ac b/configure.ac
index 8775c4b..66ac5df 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1775,6 +1775,8 @@ AC_CONFIG_FILES([Makefile
                  tests/charsets/Makefile
                  tests/create/Makefile
                  tests/data/Makefile
+                 tests/daemon/Makefile
+                 tests/daemon/captive-daemon.pm
                  tests/discard/Makefile
                  tests/disks/Makefile
                  tests/disks/test-qemu-drive-libvirt.xml
diff --git a/tests/daemon/Makefile.am b/tests/daemon/Makefile.am
new file mode 100644
index 0000000..0d2f1cd
--- /dev/null
+++ b/tests/daemon/Makefile.am
@@ -0,0 +1,36 @@
+# libguestfs
+# Copyright (C) 2015 Red Hat Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+# Tests in this subdirectory run the daemon as a captive host process
+# and send commands directly to it.  There is a small Perl library
+# called 'captive-daemon.pm' in this directory to help with this.
+
+include $(top_srcdir)/subdir-rules.mk
+
+check_DATA = captive-daemon.pm
+
+TESTS = \
+	test-daemon-start.pl
+
+TESTS_ENVIRONMENT = $(top_builddir)/run --test $(VG)
+
+EXTRA_DIST = \
+	captive-daemon.pm \
+	$(TESTS)
+
+check-valgrind:
+	$(MAKE) check TEST_WITH_VALGRIND=1
diff --git a/tests/daemon/captive-daemon.pm.in
b/tests/daemon/captive-daemon.pm.in
new file mode 100644
index 0000000..991a9a1
--- /dev/null
+++ b/tests/daemon/captive-daemon.pm.in
@@ -0,0 +1,125 @@
+# libguestfs
+# Copyright (C) 2015 Red Hat Inc.
+# @configure_input@
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+# Small library to help testing the daemon directly.
+
+package CaptiveDaemon;
+
+use strict;
+use warnings;
+
+use Sys::Guestfs;
+
+$| = 1;
+
+# Filled in by autoconf.
+my %var;
+$var{top_builddir} = "@top_builddir@";
+$var{abs_top_srcdir} = "@abs_top_srcdir@";
+$var{abs_top_builddir} = "@abs_top_builddir@";
+$var{VALGRIND} = "@VALGRIND@";
+
+# Now we have to substitute the above variables into this one:
+my $VG = '@VG@';
+$VG =~ s/\$\(([A-Za-z_]+)\)/ $var{"$1"} /ge;
+
+# Refuse to run if the user is trying to run tests as root.  There's
+# too much risk that things will go badly wrong.
+if ($> == 0) {
+    print "$0: don't run the libguestfs tests as root!\n";
+    exit 77
+}
+
+sub run_tests {
+    my $g = Sys::Guestfs->new();
+    my $tmpdir = $g->get_tmpdir;
+    my $verbose = $g->get_verbose;
+    $g->close;
+
+    # Choose a random name for the socket.  The daemon will create the
+    # socket so we don't need to do that.
+    my @chars = ("A".."Z", "a".."z",
"0".."9");
+    my $sockname = $tmpdir;
+    $sockname .= "/";
+    $sockname .= $chars[rand @chars] for 1..8;
+
+    # Assemble the command we will run in the subprocess.
+    my $cmd +        "$var{top_builddir}/daemon/guestfsd --channel
$sockname -r -t -l";
+    if ($verbose) {
+        $cmd = $cmd . " -v"
+    }
+    if ($ENV{TEST_WITH_VALGRIND}) {
+        $cmd = $VG . " " . $cmd
+    }
+
+    if ($verbose) {
+        print "$0: running: $cmd\n";
+    }
+
+    # Fork to run the daemon in the background.
+    my $pid = fork ();
+    die "fork: $!" unless defined $pid;
+    if ($pid == 0) {
+        # Child process: the daemon.
+        exec $cmd or die "guestfsd: $!";
+    }
+
+    # Wait for the daemon to create the socket, but if it doesn't
+    # appear after a short timeout, assume there has been a failure.
+    for (my $i = 0; $i < 10; ++$i) {
+        last if -S $sockname;
+        sleep 1;
+    }
+    die "subprocess did not create the socket, check earlier
messages\n"
+        unless -S $sockname;
+
+    # Create the libguestfs handle and connect to the daemon using
+    # libguestfs live.
+    $g = Sys::Guestfs->new ();
+    $g->set_backend ("unix:" . $sockname);
+    $g->launch;
+
+    # Run the user tests.
+    my $r = ::tests ($g);
+
+    # Close the socket.  The daemon should now exit.
+    $g->shutdown ();
+    $g->close ();
+    unlink $sockname;
+
+    waitpid ($pid, 0) or die "waitpid: $!";
+    if ($? != 0) {
+        my $signal = $? & 127;
+        die "ERROR: guestfsd died on signal $signal\n" if $signal;
+        my $crash = $? & 128;
+        die "ERROR: guestfsd core dumped\n" if $crash;
+        my $status = $? >> 8;
+        die "ERROR: guestfsd died with exit code 119 (valgrind
failure)\n"
+            if $status == 119;
+
+        # Note we allow guestfsd to die with exit code 1, because
+        # that indicates a read failure from the socket.
+        die "ERROR: guestfsd died with exit code $status\n" if
$status > 1;
+    }
+
+    # Exit with failure if the user test failed.
+    exit 1 unless $r
+}
+
+1;
diff --git a/tests/daemon/test-daemon-start.pl
b/tests/daemon/test-daemon-start.pl
new file mode 100755
index 0000000..91e008a
--- /dev/null
+++ b/tests/daemon/test-daemon-start.pl
@@ -0,0 +1,35 @@
+#!/usr/bin/perl -w
+# libguestfs
+# Copyright (C) 2015 Red Hat Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+# Test that the daemon starts and stops.
+
+use strict;
+use warnings;
+
+require 'captive-daemon.pm';
+
+sub tests {
+    my $g = shift;
+
+    $g->ping_daemon;
+
+    # Return true to indicate the test succeeded.
+    1;
+}
+
+CaptiveDaemon::run_tests ()
-- 
2.3.1

This refines the previous commit by shutting down the daemon cleanly
at the end of the test (assuming the test was successful).  It
repurposes the 'internal_exit' API for this, which was previously used
by the now defunct --enable-valgrind-daemon functionality.
---
 daemon/internal.c                 |  7 +------
 generator/actions.ml              |  3 ++-
 tests/daemon/captive-daemon.pm.in | 12 +++++-------
 3 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/daemon/internal.c b/daemon/internal.c
index 781366d..5fb3d54 100644
--- a/daemon/internal.c
+++ b/daemon/internal.c
@@ -46,14 +46,9 @@ do_internal_autosync (void)
 }
 
 /* NB: Only called when valgrinding the daemon. */
-int
+int __attribute__((noreturn))
 do_internal_exit (void)
 {
-  if (!autosync_umount) {
-    reply_with_error ("guestfsd -r flag used, ignoring");
-    return -1;
-  }
-
   /* Send a reply before exiting so the protocol doesn't get confused. */
   reply (NULL, NULL);
 
diff --git a/generator/actions.ml b/generator/actions.ml
index 24e84b5..a48d8ce 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -11951,7 +11951,8 @@ This function is used internally when setting up the
appliance." };
     name = "internal_exit"; added = (1, 23, 30);
     style = RErr, [], [];
     proc_nr = Some 414;
-    visibility = VInternal;
+    (* Really VInternal, but we need to use it from the Perl bindings. XXX *)
+    visibility = VDebug;
     cancellable = true;
     shortdesc = "cause the daemon to exit (internal use only)";
     longdesc = "\
diff --git a/tests/daemon/captive-daemon.pm.in
b/tests/daemon/captive-daemon.pm.in
index 991a9a1..19833b8 100644
--- a/tests/daemon/captive-daemon.pm.in
+++ b/tests/daemon/captive-daemon.pm.in
@@ -93,14 +93,15 @@ sub run_tests {
     # libguestfs live.
     $g = Sys::Guestfs->new ();
     $g->set_backend ("unix:" . $sockname);
+    $g->set_autosync (0);
     $g->launch;
 
     # Run the user tests.
     my $r = ::tests ($g);
 
-    # Close the socket.  The daemon should now exit.
-    $g->shutdown ();
-    $g->close ();
+    # Tell the daemon to exit cleanly, and remove the socket.
+    $g->internal_exit;
+    $g->close;
     unlink $sockname;
 
     waitpid ($pid, 0) or die "waitpid: $!";
@@ -112,10 +113,7 @@ sub run_tests {
         my $status = $? >> 8;
         die "ERROR: guestfsd died with exit code 119 (valgrind
failure)\n"
             if $status == 119;
-
-        # Note we allow guestfsd to die with exit code 1, because
-        # that indicates a read failure from the socket.
-        die "ERROR: guestfsd died with exit code $status\n" if
$status > 1;
+        die "ERROR: guestfsd died with exit code $status\n";
     }
 
     # Exit with failure if the user test failed.
-- 
2.3.1

---
 daemon/btrfs.c             |  3 +-
 tests/daemon/Makefile.am   |  3 +-
 tests/daemon/test-btrfs.pl | 69 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 73 insertions(+), 2 deletions(-)
 create mode 100755 tests/daemon/test-btrfs.pl

diff --git a/daemon/btrfs.c b/daemon/btrfs.c
index 69e306b..7e29e9d 100644
--- a/daemon/btrfs.c
+++ b/daemon/btrfs.c
@@ -42,7 +42,8 @@ GUESTFSD_EXT_CMD(str_btrfsimage, btrfs-image);
 int
 optgroup_btrfs_available (void)
 {
-  return prog_exists (str_btrfs) && filesystem_available
("btrfs") > 0;
+  return test_mode ||
+    (prog_exists (str_btrfs) && filesystem_available
("btrfs") > 0);
 }
 
 char *
diff --git a/tests/daemon/Makefile.am b/tests/daemon/Makefile.am
index 0d2f1cd..bb380c5 100644
--- a/tests/daemon/Makefile.am
+++ b/tests/daemon/Makefile.am
@@ -24,7 +24,8 @@ include $(top_srcdir)/subdir-rules.mk
 check_DATA = captive-daemon.pm
 
 TESTS = \
-	test-daemon-start.pl
+	test-daemon-start.pl \
+	test-btrfs.pl
 
 TESTS_ENVIRONMENT = $(top_builddir)/run --test $(VG)
 
diff --git a/tests/daemon/test-btrfs.pl b/tests/daemon/test-btrfs.pl
new file mode 100755
index 0000000..fff927e
--- /dev/null
+++ b/tests/daemon/test-btrfs.pl
@@ -0,0 +1,69 @@
+#!/usr/bin/perl -w
+# libguestfs
+# Copyright (C) 2015 Red Hat Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+# Test that the daemon starts and stops.
+
+use strict;
+use warnings;
+
+use File::Temp qw/tempdir/;
+
+require 'captive-daemon.pm';
+
+# Set $PATH to include directory that will have phony 'btrfs' binary.
+my $bindir = tempdir (CLEANUP => 1);
+$ENV{PATH} = $bindir . ":" . $ENV{PATH};
+
+sub set_btrfs_output {
+    my $output = shift;
+    open BTRFS, ">$bindir/btrfs" or die "$bindir/btrfs:
$!";
+    print BTRFS "#!/bin/sh\n";
+    print BTRFS "cat << '__EOF'\n";
+    print BTRFS $output;
+    print BTRFS "__EOF\n";
+    close BTRFS;
+    chmod 0755, "$bindir/btrfs" or die "chmod: $bindir/btrfs:
$!";
+}
+
+sub tests {
+    my $g = shift;
+
+    # Test btrfs_subvolume_list.
+    my $output = <<EOF;
+ID 256 gen 30 top level 5 path test1
+ID 257 gen 30 top level 5 path dir/test2
+ID 258 gen 30 top level 5 path test3
+EOF
+    set_btrfs_output ($output);
+    my @r = $g->btrfs_subvolume_list ("/");
+    die unless @r == 3;
+    die unless $r[0]->{btrfssubvolume_id} == 256;
+    die unless $r[0]->{btrfssubvolume_top_level_id} == 5;
+    die unless $r[0]->{btrfssubvolume_path} eq "test1";
+    die unless $r[1]->{btrfssubvolume_id} == 257;
+    die unless $r[1]->{btrfssubvolume_top_level_id} == 5;
+    die unless $r[1]->{btrfssubvolume_path} eq "dir/test2";
+    die unless $r[2]->{btrfssubvolume_id} == 258;
+    die unless $r[2]->{btrfssubvolume_top_level_id} == 5;
+    die unless $r[2]->{btrfssubvolume_path} eq "test3";
+
+    # Return true to indicate the test succeeded.
+    1;
+}
+
+CaptiveDaemon::run_tests ()
-- 
2.3.1

---
 tests/daemon/test-btrfs.pl | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/tests/daemon/test-btrfs.pl b/tests/daemon/test-btrfs.pl
index fff927e..815ab1d 100755
--- a/tests/daemon/test-btrfs.pl
+++ b/tests/daemon/test-btrfs.pl
@@ -62,6 +62,19 @@ EOF
     die unless $r[2]->{btrfssubvolume_top_level_id} == 5;
     die unless $r[2]->{btrfssubvolume_path} eq "test3";
 
+    # Test btrfs_qgroup_show.
+    my $output = <<EOF;
+qgroupid rfer excl 
+-------- ---- ---- 
+0/5      4096 4096 
+EOF
+    set_btrfs_output ($output);
+    my @r = $g->btrfs_qgroup_show ("/");
+    die unless @r == 1;
+    die unless $r[0]->{btrfsqgroup_id} == "0/5";
+    die unless $r[0]->{btrfsqgroup_rfer} == 4096;
+    die unless $r[0]->{btrfsqgroup_excl} == 4096;
+
     # Return true to indicate the test succeeded.
     1;
 }
-- 
2.3.1

In data giovedì 25 giugno 2015 15:56:56, Richard W.M. Jones ha
scritto:
> This option, used for testing, causes the daemon to create the Unix
> domain socket (from guestfs_channel), listen on it, and accept a
> single connection.
> ---
> [...]
> +  else {
> +    struct sockaddr_un addr;
> +
> +    sock = socket (AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0);
> +    if (sock == -1)
> +      error (EXIT_FAILURE, errno, "socket");
> +    addr.sun_family = AF_UNIX;
> +    strncpy (addr.sun_path, channel, UNIX_PATH_MAX);
> +    addr.sun_path[UNIX_PATH_MAX-1] = '\0';
As a safety measure, would be better to error() if strlen(channel) is
greater than UNIX_PATH_MAX-1 (avoid truncating the socket path passed).

Thanks,
-- 
Pino Toscano

In data giovedì 25 giugno 2015 15:56:58, Richard W.M. Jones ha
scritto:
> This allows us to test the daemon running as a host process, allowing
> us to meaningfully test it using valgrind.
> 
> This commit only adds a single test that check that the daemon starts
> up, can be pinged, and exits.
> ---
> [...]> +sub run_tests {
> +    my $g = Sys::Guestfs->new();
> +    my $tmpdir = $g->get_tmpdir;
> +    my $verbose = $g->get_verbose;
On this throw-away handle we could check whether the unix backend is
built in (unlike in RHEL, for example):

 eval {
     $g->set_backend ("unix:/idontexist");
 };
 if ($@) {
     warn "$0: skipping test because the 'unix' backend not
available\n";
     exit 77;
 };

Thanks,
-- 
Pino Toscano

In data giovedì 25 giugno 2015 15:56:53, Richard W.M. Jones ha
scritto:
> 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.
> ---
I understand you don't like how it is currently done (and I might
agree); OTOH, some of the stuff here could still be used when trying to
valgrind the daemon within the appliance. Can you please leave this
commit aside for now, while I'm expermenting with a simplier valgrind
mode for the daemon?

Thanks,
-- 
Pino Toscano

In data giovedì 25 giugno 2015 15:56:59, Richard W.M. Jones ha
scritto:
> This refines the previous commit by shutting down the daemon cleanly
> at the end of the test (assuming the test was successful).  It
> repurposes the 'internal_exit' API for this, which was previously
used
> by the now defunct --enable-valgrind-daemon functionality.
> ---
>  daemon/internal.c                 |  7 +------
>  generator/actions.ml              |  3 ++-
>  tests/daemon/captive-daemon.pm.in | 12 +++++-------
>  3 files changed, 8 insertions(+), 14 deletions(-)
> 
> diff --git a/daemon/internal.c b/daemon/internal.c
> index 781366d..5fb3d54 100644
> --- a/daemon/internal.c
> +++ b/daemon/internal.c
> @@ -46,14 +46,9 @@ do_internal_autosync (void)
>  }
>  
>  /* NB: Only called when valgrinding the daemon. */
> -int
> +int __attribute__((noreturn))
>  do_internal_exit (void)
>  {
> -  if (!autosync_umount) {
> -    reply_with_error ("guestfsd -r flag used, ignoring");
> -    return -1;
> -  }
> -
>    /* Send a reply before exiting so the protocol doesn't get confused.
*/
>    reply (NULL, NULL);
>  
> diff --git a/generator/actions.ml b/generator/actions.ml
> index 24e84b5..a48d8ce 100644
> --- a/generator/actions.ml
> +++ b/generator/actions.ml
> @@ -11951,7 +11951,8 @@ This function is used internally when setting up
the appliance." };
>      name = "internal_exit"; added = (1, 23, 30);
>      style = RErr, [], [];
>      proc_nr = Some 414;
> -    visibility = VInternal;
> +    (* Really VInternal, but we need to use it from the Perl bindings. XXX
*)
> +    visibility = VDebug;
>      cancellable = true;
>      shortdesc = "cause the daemon to exit (internal use only)";
>      longdesc = "\
An option here could be turning internal_exit from a daemon API into
a debug subcommand. It would be undocumented as it should, not leaving
it exposed more.

Thanks,
-- 
Pino Toscano