Richard W.M. Jones
2015-Jun-25 14:56 UTC
[Libguestfs] [PATCH v2 0/9] Better testing of the guestfsd 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.
Richard W.M. Jones
2015-Jun-25 14:56 UTC
[Libguestfs] [PATCH v2 1/9] build: Remove ./configure --enable-valgrind-daemon.
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
Richard W.M. Jones
2015-Jun-25 14:56 UTC
[Libguestfs] [PATCH v2 2/9] configure: Remove test for valgrind --vgdb option.
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
Richard W.M. Jones
2015-Jun-25 14:56 UTC
[Libguestfs] [PATCH v2 3/9] daemon: Add undocumented --test / -t option to enable test mode.
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
Richard W.M. Jones
2015-Jun-25 14:56 UTC
[Libguestfs] [PATCH v2 4/9] daemon: Add -l / --listen flag.
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
Richard W.M. Jones
2015-Jun-25 14:56 UTC
[Libguestfs] [PATCH v2 5/9] daemon: Move all the kernel command line parsing to the init script.
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
Richard W.M. Jones
2015-Jun-25 14:56 UTC
[Libguestfs] [PATCH v2 6/9] tests: Add tests using a captive daemon process.
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
Richard W.M. Jones
2015-Jun-25 14:56 UTC
[Libguestfs] [PATCH v2 7/9] tests: daemon: Cleanly shut down the daemon on exit.
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
Richard W.M. Jones
2015-Jun-25 14:57 UTC
[Libguestfs] [PATCH v2 8/9] tests: Add a test of btrfs-subvolume-list using the captive daemon.
--- 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
Richard W.M. Jones
2015-Jun-25 14:57 UTC
[Libguestfs] [PATCH v2 9/9] tests: Add a test of btrfs-qgroup-show using the captive daemon.
--- 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
Pino Toscano
2015-Jun-29 16:49 UTC
Re: [Libguestfs] [PATCH v2 4/9] daemon: Add -l / --listen flag.
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
Pino Toscano
2015-Jun-29 17:11 UTC
Re: [Libguestfs] [PATCH v2 6/9] tests: Add tests using a captive daemon process.
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
Pino Toscano
2015-Jun-29 17:17 UTC
Re: [Libguestfs] [PATCH v2 1/9] build: Remove ./configure --enable-valgrind-daemon.
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
Pino Toscano
2015-Jun-29 17:17 UTC
Re: [Libguestfs] [PATCH v2 7/9] tests: daemon: Cleanly shut down the daemon on exit.
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
Possibly Parallel Threads
- [PATCH 0/7] Better testing of the guestfsd daemon.
- [PATCH] [repost] build: Remove ./configure --enable-valgrind-daemon.
- [PATCH 00/14] Run the daemon under valgrind and fix resultant errors.
- [PATCH 0/4] Not quite working User-Mode Linux backend.
- [PATCH v2 0/4] Experimental User-Mode Linux backend.