[Libguestfs] [PATCH v2] fuse: Add guestunmount program to handle unmounting (RHBZ#916780).

From: "Richard W.M. Jones" <rjones(a)redhat.com&gt; --- .gitignore | 5 + fish/test-mount-local.sh | 10 +- fuse/Makefile.am | 85 ++++++- fuse/guestmount.pod | 52 ++-- fuse/guestunmount.c | 346 ++++++++++++++++++++++++++ fuse/guestunmount.pod | 164 ++++++++++++ fuse/test-fuse-umount-race.sh | 11 +- fuse/test-fuse.sh | 25 +- fuse/test-guestunmount-fd.c | 122 +++++++++ fuse/test-guestunmount-not-mounted.sh | 50 ++++ ocaml/t/guestfs_500_mount_local.ml | 37 +-- po-docs/ja/Makefile.am | 1 + po-docs/podfiles | 1 + po-docs/uk/Makefile.am | 1 + po/POTFILES | 2 + sysprep/Makefile.am | 1 + sysprep/sysprep_operation_script.ml | 14 +- tests/mount-local/test-parallel-mount-local.c | 53 ++-- tests/selinux/run-test.pl | 23 +- 19 files changed, 838 insertions(+), 165 deletions(-) create mode 100644 fuse/guestunmount.c create mode 100644 fuse/guestunmount.pod create mode 100644 fuse/test-guestunmount-fd.c create mode 100755 fuse/test-guestunmount-not-mounted.sh diff --git a/.gitignore b/.gitignore index 0747430..c49658d 100644 --- a/.gitignore +++ b/.gitignore @@ -141,7 +141,11 @@ Makefile.in /format/virt-format.1 /fuse/guestmount /fuse/guestmount.1 +/fuse/guestunmount +/fuse/guestunmount.1 /fuse/stamp-guestmount.pod +/fuse/stamp-guestunmount.pod +/fuse/test-guestunmount-fd /generator/.depend /generator/files-generated.txt /generator/generator @@ -181,6 +185,7 @@ Makefile.in /html/guestfs-testing.1.html /html/guestfsd.8.html /html/guestmount.1.html +/html/guestunmount.1.html /html/libguestfs-make-fixed-appliance.1.html /html/libguestfs-test-tool.1.html /html/virt-alignment-scan.1.html diff --git a/fish/test-mount-local.sh b/fish/test-mount-local.sh index ad62a92..8845c7c 100755 --- a/fish/test-mount-local.sh +++ b/fish/test-mount-local.sh @@ -1,6 +1,6 @@ #!/bin/bash - # libguestfs -# Copyright (C) 2012 Red Hat Inc. +# Copyright (C) 2012-2013 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 @@ -50,12 +50,8 @@ if [ $# -gt 0 -a "$1" = "--run-test" ]; then echo 'mount-local test successful' > mp/ok - # Unmount the mountpoint. Might need to retry this. - count=10 - while ! fusermount -u mp && [ $count -gt 0 ]; do - sleep 1 - ((count--)) - done + # Unmount the mountpoint. + ../fuse/guestunmount mp exit 0 fi diff --git a/fuse/Makefile.am b/fuse/Makefile.am index 12905a3..56f4cf4 100644 --- a/fuse/Makefile.am +++ b/fuse/Makefile.am @@ -17,13 +17,22 @@ include $(top_srcdir)/subdir-rules.mk -EXTRA_DIST = guestmount.pod test-fuse.sh test-fuse-umount-race.sh +EXTRA_DIST = \ + guestmount.pod \ + guestunmount.pod \ + test-fuse.sh \ + test-fuse-umount-race.sh \ + test-guestunmount-not-mounted.sh -CLEANFILES = stamp-guestmount.pod +CLEANFILES = \ + stamp-guestmount.pod \ + stamp-guestunmount.pod if HAVE_FUSE -bin_PROGRAMS = guestmount +bin_PROGRAMS = \ + guestmount \ + guestunmount # These source files (all related to option parsing) are shared # between guestfish and guestmount. @@ -35,6 +44,8 @@ SHARED_SOURCE_FILES = \ ../fish/options.h \ ../fish/options.c +# guestmount + guestmount_SOURCES = \ $(SHARED_SOURCE_FILES) \ guestmount.c @@ -61,10 +72,35 @@ guestmount_LDADD = \ $(LIBVIRT_LIBS) \ ../gnulib/lib/libgnu.la +# guestunmount + +guestunmount_SOURCES = \ + guestunmount.c + +guestunmount_CPPFLAGS = \ + -DLOCALEBASEDIR=\""$(datadir)/locale"\" \ + -I$(top_srcdir)/src -I$(top_builddir)/src \ + -I$(srcdir)/../gnulib/lib -I../gnulib/lib + +guestunmount_CFLAGS = \ + $(WARN_CFLAGS) $(WERROR_CFLAGS) \ + $(GPROF_CFLAGS) $(GCOV_CFLAGS) + +guestunmount_LDADD = \ + $(top_builddir)/src/libutils.la \ + $(top_builddir)/src/libguestfs.la \ + $(LIBXML2_LIBS) \ + $(LIBVIRT_LIBS) \ + ../gnulib/lib/libgnu.la + # Documentation. -man_MANS = guestmount.1 -noinst_DATA = $(top_builddir)/html/guestmount.1.html +man_MANS = \ + guestmount.1 \ + guestunmount.1 +noinst_DATA = \ + $(top_builddir)/html/guestmount.1.html \ + $(top_builddir)/html/guestunmount.1.html guestmount.1 $(top_builddir)/html/guestmount.1.html: stamp-guestmount.pod @@ -76,13 +112,50 @@ stamp-guestmount.pod: guestmount.pod $< touch $@ +guestunmount.1 $(top_builddir)/html/guestunmount.1.html: stamp-guestunmount.pod + +stamp-guestunmount.pod: guestunmount.pod + $(PODWRAPPER) \ + --man guestunmount.1 \ + --html $(top_builddir)/html/guestunmount.1.html \ + --license GPLv2+ \ + $< + touch $@ + # Tests. +TESTS = \ + test-guestunmount-fd \ + test-guestunmount-not-mounted.sh + if ENABLE_APPLIANCE -TESTS = test-fuse.sh test-fuse-umount-race.sh +TESTS += \ + test-fuse.sh \ + test-fuse-umount-race.sh endif ENABLE_APPLIANCE + TESTS_ENVIRONMENT = \ top_builddir=.. \ $(top_builddir)/run --test +check_PROGRAMS = test-guestunmount-fd + +test_guestunmount_fd_SOURCES = \ + test-guestunmount-fd.c + +test_guestunmount_fd_CPPFLAGS = \ + -I$(top_srcdir)/src -I$(top_builddir)/src \ + -I$(srcdir)/../gnulib/lib -I../gnulib/lib + +test_guestunmount_fd_CFLAGS = \ + $(WARN_CFLAGS) $(WERROR_CFLAGS) \ + $(GPROF_CFLAGS) $(GCOV_CFLAGS) + +test_guestunmount_fd_LDADD = \ + $(top_builddir)/src/libutils.la \ + $(top_builddir)/src/libguestfs.la \ + $(LIBXML2_LIBS) \ + $(LIBVIRT_LIBS) \ + ../gnulib/lib/libgnu.la + endif HAVE_FUSE diff --git a/fuse/guestmount.pod b/fuse/guestmount.pod index eb32684..37c85eb 100644 --- a/fuse/guestmount.pod +++ b/fuse/guestmount.pod @@ -33,8 +33,8 @@ manual page, or by looking at the examples below. FUSE lets you mount filesystems as non-root. The mountpoint must be owned by you, and the filesystem will not be visible to any other users unless you make certain global configuration changes to -C</etc/fuse.conf>. To unmount the filesystem, use the C<fusermount -u> -command. +C</etc/fuse.conf>. To unmount the filesystem, use the +L<guestunmount(1)> command. =head1 EXAMPLES @@ -70,6 +70,10 @@ If you want to debug the program, we recommend: guestmount [...] --trace --verbose /mnt +To unmount the filesystem after using it: + + guestunmount /mnt + =head1 NOTES =head2 Other users cannot see the filesystem by default @@ -96,22 +100,11 @@ the mountpoint you have just created, holding it open and preventing you from unmounting it. The usual culprits are various GUI "indexing" programs. -The popular workaround for this problem is to retry the -C<fusermount -u> command a few times until it works. Unfortunately -this isn't a reliable fix if (for example) the mounted filesystem is -particularly large and the intruding program particularly persistent. - - timeout=10 - - count=$timeout - while ! fusermount -u $mountpoint && [ $count -gt 0 ]; do - sleep 1 - ((count--)) - done - if [ $count -eq 0 ]; then - echo "$0: fusermount failed after $timeout seconds" - exit 1 - fi +The popular workaround for this problem is to retry the C<fusermount -u> +command a few times until it works (L<guestunmount(1)> does this +for you). Unfortunately this isn't a reliable fix if (for example) +the mounted filesystem is particularly large and the intruding program +particularly persistent. A proper fix is to use a private mountpoint by creating a new mount namespace using the Linux-specific L<clone(2)>/L<unshare(2)> flag @@ -120,35 +113,34 @@ would also probably need to add it as a feature to guestmount. =head2 Race conditions possible when shutting down the connection -When C<fusermount -u> exits, guestmount may still be running and -cleaning up the mountpoint. The disk image will not be fully -finalized. +When L<guestunmount(1)>/L<fusermount(1)> exits, guestmount may still +be running and cleaning up the mountpoint. The disk image will not be +fully finalized. This means that scripts like the following have a nasty race condition: guestmount -a disk.img -i /mnt # copy things into /mnt - fusermount -u /mnt + guestunmount /mnt # immediately try to use 'disk.img' ** UNSAFE ** The solution is to use the I<--pid-file> option to write the -guestmount PID to a file, then after fusermount spin waiting for this -PID to exit. +guestmount PID to a file, then after guestunmount spin waiting for +this PID to exit. guestmount -a disk.img -i --pid-file guestmount.pid /mnt # ... # ... - # Save the PID of guestmount *before* calling fusermount. + # Save the PID of guestmount *before* calling guestunmount. pid="$(cat guestmount.pid)" - timeout=10 + # Unmount the filesystem. + guestunmount /mnt - # fusermount retry code, see above - # ... - # ... + timeout=10 count=$timeout while kill -0 "$pid" 2>/dev/null && [ $count -gt 0 ]; do @@ -397,6 +389,8 @@ error. =head1 SEE ALSO +L<guestunmount(1)>, +L<fusermount(1)>, L<guestfish(1)>, L<virt-inspector(1)>, L<virt-cat(1)>, diff --git a/fuse/guestunmount.c b/fuse/guestunmount.c new file mode 100644 index 0000000..2411521 --- /dev/null +++ b/fuse/guestunmount.c @@ -0,0 +1,346 @@ +/* guestunmount + * Copyright (C) 2013 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. + */ + +#include <config.h> + +#include <stdio.h> +#include <stdlib.h> +#include <stdint.h> +#include <stdbool.h> +#include <inttypes.h> +#include <string.h> +#include <unistd.h> +#include <getopt.h> +#include <errno.h> +#include <locale.h> +#include <libintl.h> +#include <poll.h> +#include <sys/types.h> +#include <sys/wait.h> + +#include "guestfs.h" +#include "guestfs-internal-frontend.h" + +#include "ignore-value.h" +#include "progname.h" + +static int do_fusermount (const char *mountpoint, char **error_rtn); +static void do_fuser (const char *mountpoint); + +static bool quiet = false; +static size_t retries = 5; +static bool verbose = false; + +static void __attribute__((noreturn)) +usage (int status) +{ + if (status != EXIT_SUCCESS) + fprintf (stderr, _("Try `%s --help' for more information.\n"), + program_name); + else { + fprintf (stdout, + _("%s: clean up a mounted filesystem\n" + "Copyright (C) 2013 Red Hat Inc.\n" + "Usage:\n" + " %s [--fd=FD] mountpoint\n" + "Options:\n" + " --fd=FD Pipe file descriptor to monitor\n" + " --help Display help message and exit\n" + " -q|--quiet Don't print fusermount errors\n" + " --no-retry Don't retry fusermount\n" + " --retry=N Retry fusermount N times (default: 5)\n" + " -v|--verbose Verbose messages\n" + " -V|--version Display version and exit\n" + ), + program_name, program_name); + } + exit (status); +} + +int +main (int argc, char *argv[]) +{ + enum { HELP_OPTION = CHAR_MAX + 1 }; + + static const char *options = "qv?V"; + static const struct option long_options[] = { + { "fd", 1, 0, 0 }, + { "help", 0, 0, HELP_OPTION }, + { "quiet", 0, 0, 'q' }, + { "no-retry", 0, 0, 0 }, + { "retry", 1, 0, 0 }, + { "verbose", 0, 0, 'v' }, + { "version", 0, 0, 'V' }, + { 0, 0, 0, 0 } + }; + + int c, fd = -1; + int option_index; + const char *mountpoint; + struct sigaction sa; + struct pollfd pollfd; + char *error = NULL; + size_t i; + + setlocale (LC_ALL, ""); + bindtextdomain (PACKAGE, LOCALEBASEDIR); + textdomain (PACKAGE); + + /* Set global program name that is not polluted with libtool artifacts. */ + set_program_name (argv[0]); + + for (;;) { + c = getopt_long (argc, argv, options, long_options, &option_index); + if (c == -1) break; + + switch (c) { + case 0: /* options which are long only */ + if (STREQ (long_options[option_index].name, "fd")) { + if (sscanf (optarg, "%d", &fd) != 1 || fd < 0) { + fprintf (stderr, _("%s: cannot parse fd option '%s'\n"), + program_name, optarg); + exit (EXIT_FAILURE); + } + } else if (STREQ (long_options[option_index].name, "no-retry")) { + retries = 0; + } else if (STREQ (long_options[option_index].name, "retry")) { + if (sscanf (optarg, "%zu", &retries) != 1 || retries >= 64) { + fprintf (stderr, _("%s: cannot parse retries option or value is too large '%s'\n"), + program_name, optarg); + exit (EXIT_FAILURE); + } + } else { + fprintf (stderr, _("%s: unknown long option: %s (%d)\n"), + program_name, long_options[option_index].name, option_index); + exit (EXIT_FAILURE); + } + break; + + case 'q': + quiet = true; + break; + + case 'v': + verbose = true; + break; + + case 'V': + printf ("guestunmount %s %s\n", PACKAGE_NAME, PACKAGE_VERSION); + exit (EXIT_SUCCESS); + + case HELP_OPTION: + usage (EXIT_SUCCESS); + + default: + usage (EXIT_FAILURE); + } + } + + /* We'd better have a mountpoint. */ + if (optind+1 != argc) { + fprintf (stderr, + _("%s: you must specify a mountpoint in the host filesystem\n"), + program_name); + exit (EXIT_FAILURE); + } + + mountpoint = argv[optind]; + + /* Monitor the pipe until we get POLLHUP. */ + if (fd >= 0) { + ignore_value (chdir ("/")); + + /* Ignore keyboard signals. */ + memset (&sa, 0, sizeof sa); + sa.sa_handler = SIG_IGN; + sa.sa_flags = SA_RESTART; + sigaction (SIGINT, &sa, NULL); + sigaction (SIGQUIT, &sa, NULL); + + while (1) { + pollfd.fd = fd; + pollfd.events = POLLIN; + pollfd.revents = 0; + if (poll (&pollfd, 1, -1) == -1) { + if (errno != EAGAIN && errno != EINTR) { + perror ("poll"); + exit (EXIT_FAILURE); + } + } + else { + if ((pollfd.revents & POLLHUP) != 0) + break; + } + } + } + + /* Unmount the filesystem. We may have to try a few times. */ + for (i = 0; i <= retries; ++i) { + if (i > 0) + sleep (1 << (i-1)); + + free (error); + error = NULL; + + if (do_fusermount (mountpoint, &error) == 0) + goto done; + + /* Did fusermount fail because the mountpoint is not mounted? */ + if (error && + strstr (error, "fusermount: entry for") != NULL) { + goto not_mounted; + } + } + + /* fusermount failed after N retries */ + if (!quiet) { + fprintf (stderr, _("%s: failed to unmount %s: %s\n"), + program_name, mountpoint, error); + do_fuser (mountpoint); + } + free (error); + + exit (2); + + /* not mounted */ + not_mounted: + if (!quiet) + fprintf (stderr, _("%s: %s is not mounted: %s\n"), + program_name, mountpoint, error); + + free (error); + + exit (2); + + /* success */ + done: + exit (EXIT_SUCCESS); +} + +static int +do_fusermount (const char *mountpoint, char **error_rtn) +{ + int fd[2]; + pid_t pid; + int r; + char *buf = NULL; + size_t allocsize = 0, len = 0; + + if (pipe (fd) == -1) { + perror ("pipe"); + exit (EXIT_FAILURE); + } + + pid = fork (); + if (pid == -1) { + perror ("fork"); + exit (EXIT_FAILURE); + } + + if (pid == 0) { /* Child - run fusermount. */ + close (fd[0]); + dup2 (fd[1], 1); + dup2 (fd[1], 2); + close (fd[1]); + + /* We have to parse error messages from fusermount, so ... */ + setenv ("LC_ALL", "C", 1); + + execlp ("fusermount", "fusermount", "-u", mountpoint, NULL); + perror ("exec"); + _exit (EXIT_FAILURE); + } + + /* Parent - read from the pipe any errors etc. */ + close (fd[1]); + + while (1) { + if (len >= allocsize) { + allocsize += 256; + buf = realloc (buf, allocsize); + if (buf == NULL) { + perror ("realloc"); + exit (EXIT_FAILURE); + } + } + + /* Leave space in the buffer for a terminating \0 character. */ + r = read (fd[0], &buf[len], allocsize - len - 1); + if (r == -1) { + perror ("read"); + exit (EXIT_FAILURE); + } + + if (r == 0) + break; + + len += r; + } + + if (close (fd[0]) == -1) { + perror ("close"); + exit (EXIT_FAILURE); + } + + if (buf) { + /* Remove any trailing \n from the error message. */ + while (len > 0 && buf[len-1] == '\n') { + buf[len-1] = '\0'; + len--; + } + + /* Make sure the error message is \0 terminated. */ + if (len < allocsize) + buf[len] = '\0'; + } + + if (waitpid (pid, &r, 0) == -1) { + perror ("waitpid"); + exit (EXIT_FAILURE); + } + + if (!WIFEXITED (r) || WEXITSTATUS (r) != 0) { + *error_rtn = buf; + return 1; /* fusermount or exec failed */ + } + + free (buf); + return 0; /* fusermount successful */ +} + +/* Try running 'fuser' on the mountpoint. This is for information + * only so don't fail if we can't run it. + */ +static void +do_fuser (const char *mountpoint) +{ + pid_t pid; + + pid = fork (); + if (pid == -1) { + perror ("fork"); + exit (EXIT_FAILURE); + } + + if (pid == 0) { /* Child - run /sbin/fuser. */ + execlp ("/sbin/fuser", "fuser", "-v", "-m", mountpoint, NULL); + _exit (EXIT_FAILURE); + } + + waitpid (pid, NULL, 0); +} diff --git a/fuse/guestunmount.pod b/fuse/guestunmount.pod new file mode 100644 index 0000000..2038e77 --- /dev/null +++ b/fuse/guestunmount.pod @@ -0,0 +1,164 @@ +=encoding utf8 + +=head1 NAME + +guestunmount - Unmount a guestmounted filesystem + +=head1 SYNOPSIS + + guestunmount mountpoint + + guestunmount --fd=<FD> mountpoint + +=head1 DESCRIPTION + +guestunmount is a utility to clean up mounted filesystems +automatically. L<guestmount(1)> mounts filesystems using libguestfs. +This program unmounts the filesystem when a program or script has finished +with it. + +guestunmount is a wrapper around the FUSE L<fusermount(1)> program, +which must exist on the current C<PATH>. + +There are two ways to use guestunmount. When called as: + + guestunmount mountpoint + +it unmounts C<mountpoint> immediately. + +When called as: + + guestunmount --fd=FD mountpoint + +it waits until the pipe C<FD> is closed. This can be used to monitor +another process and clean up its mountpoint when that process exits, +as described below. + +=head2 FROM PROGRAMS + +You can just call C<guestunmount mountpoint> from the program, but a +more sophisticated way to use guestunmount is to have it monitor your +program so it can clean up the mount point if your program exits +unexpectedly. + +In the program, create a pipe (eg. by calling L<pipe(2)>). Let C<FD> +be the file descriptor number of the read side of the pipe +(ie. C<pipefd[0]>). + +After mounting the filesystem with L<guestmount(1)> (on +C<mountpoint>), fork and run guestunmount like this: + + guestunmount --fd=FD mountpoint + +Close the read side of the pipe in the parent process. + +Now, when the write side of the pipe (ie. C<pipefd[1]>) is closed for +any reason, either explicitly or because the parent process +exits, guestunmount notices and unmounts the mountpoint. + +If your operating system supports it, you should set the C<FD_CLOEXEC> +flag on the write side of the pipe. This is so that other child +processes don't inherit the file descriptor and keep it open. + +Guestunmount never daemonizes itself. + +=head2 FROM SHELL SCRIPTS + +Since bash doesn't provide a way to create an unnamed pipe, use a trap +to call guestunmount on exit like this: + + trap "guestunmount mountpoint" EXIT INT QUIT TERM + +=head1 OPTIONS + +=over 4 + +=item B<--fd=FD> + +Specify the pipe file descriptor to monitor, and delay cleanup until +that pipe is closed. + +=item B<--help> + +Display brief help and exit. + +=item B<-q> + +=item B<--quiet> + +Don't display error messages from fusermount. The return status is +still set (see L</EXIT STATUS> below). + +=item B<--no-retry> + +=item B<--retry=N> + +By default, guestunmount will retry the fusermount operation up to +S<5 times> (that is, it will run it up to S<6 times> = S<1 try> + +S<5 retries>). + +Use I<--no-retry> to make guestunmount run fusermount only once. + +Use I<--retry=N> to make guestunmount retry C<N> times instead of 5. + +guestunmount performs an exponential back-off between retries, waiting +S<1 second>, S<2 seconds>, S<4 seconds>, etc before each retry. + +=item B<-V> + +=item B<--version> + +Display the program version and exit. + +=back + +=head1 ENVIRONMENT VARIABLES + +=over 4 + +=item C<PATH> + +The L<fusermount(1)> program (supplied by FUSE) must be available on +the current C<PATH>. + +=back + +=head1 EXIT STATUS + +This program returns 0 if successful, or one of the following error +codes: + +=over 4 + +=item C<1> + +Program error, eg. could not allocate memory, could not run fusermount. +See the error message printed for more information. + +=item C<2> + +The mount point could not be unmounted even after retrying. See +the error message printed for the underlying fusermount error. + +=item C<3> + +The mount point is not mounted. + +=back + +=head1 SEE ALSO + +L<guestmount(1)>, +L<fusermount(1)>, +L<pipe(2)>, +L<guestfs(3)/MOUNT LOCAL>, +L<http://libguestfs.org/>, +L<http://fuse.sf.net/>. + +=head1 AUTHORS + +Richard W.M. Jones (C<rjones at redhat dot com>) + +=head1 COPYRIGHT + +Copyright (C) 2013 Red Hat Inc. diff --git a/fuse/test-fuse-umount-race.sh b/fuse/test-fuse-umount-race.sh index 59186ab..52912ef 100755 --- a/fuse/test-fuse-umount-race.sh +++ b/fuse/test-fuse-umount-race.sh @@ -48,15 +48,8 @@ pid="$(cat test.pid)" timeout=10 -count=$timeout -while ! fusermount -u mp && [ $count -gt 0 ]; do - sleep 1 - ((count--)) -done -if [ $count -eq 0 ]; then - echo "$0: fusermount failed after $timeout seconds" - exit 1 -fi +# Unmount the mountpoint. +./guestunmount mp # Wait for guestmount to exit. count=$timeout diff --git a/fuse/test-fuse.sh b/fuse/test-fuse.sh index bd0c096..f17ff22 100755 --- a/fuse/test-fuse.sh +++ b/fuse/test-fuse.sh @@ -49,18 +49,24 @@ top_builddir=$(cd "$top_builddir" > /dev/null; pwd) # Paths to the other programs and files. NB: Must be absolute paths. guestfish="$top_builddir/fish/guestfish" guestmount="$top_builddir/fuse/guestmount" +guestunmount="$top_builddir/fuse/guestunmount" image="$top_builddir/fuse/test.img" mp="$top_builddir/fuse/test-mp" -if [ ! -x "$guestfish" -o ! -x "$guestmount" ]; then - echo "$0: error: guestfish or guestmount are not available" +if [ ! -x "$guestfish" -o ! -x "$guestmount" -o ! -x "$guestunmount" ] +then + echo "$0: error: guestfish, guestmount or guestunmount are not available" exit 1 fi -# Ensure everything is cleaned up on exit. +# Ensure the mountpoint directory exists and is not being used. rm -f "$image" mkdir -p "$mp" fusermount -u "$mp" >/dev/null 2>&1 ||: + +# Ensure everything is cleaned up on exit. +mounted= + function cleanup () { status=$? @@ -75,15 +81,9 @@ function cleanup () # Who's using this? Should be no one, but see below. if [ -x /sbin/fuser ]; then /sbin/fuser "$mp"; fi - # If you run this and you have GNOME running at the same time, - # then randomly /usr/libexec/gvfs-gdu-volume-monitor will decide - # to do whatever it does in the mountpoint directory, preventing - # you from unmounting it! Hence the need for this loop. - count=10 - while ! fusermount -u "$mp" && [ $count -gt 0 ]; do - sleep 1 - ((count--)) - done + if [ -n "$mounted" ]; then + $guestunmount "$mp" + fi rm -f "$image" rm -rf "$mp" @@ -121,6 +121,7 @@ $guestmount \ -o uid="$(id -u)" -o gid="$(id -g)" "$mp" # To debug guestmount, add this to the end of the preceding command: # -v -x & sleep 60 +mounted=yes stage Changing into mounted directory cd "$mp" diff --git a/fuse/test-guestunmount-fd.c b/fuse/test-guestunmount-fd.c new file mode 100644 index 0000000..4661a95 --- /dev/null +++ b/fuse/test-guestunmount-fd.c @@ -0,0 +1,122 @@ +/* libguestfs + * Copyright (C) 2013 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 the guestunmount --fd flag. Note this is done without + * requiring libguestfs or guestmount. + */ + +#include <config.h> + +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <unistd.h> +#include <sys/types.h> +#include <sys/wait.h> + +#include "guestfs.h" +#include "guestfs-internal-frontend.h" + +static void display_exit_status (int status, FILE *fp); + +int +main (int argc, char *argv[]) +{ + int pipefd[2]; + pid_t pid; + int r, status; + + /* Create the pipe. */ + if (pipe (pipefd) == -1) { + perror ("pipe"); + exit (EXIT_FAILURE); + } + + /* Create the guestunmount subprocess. */ + pid = fork (); + if (pid == -1) { + perror ("fork"); + exit (EXIT_FAILURE); + } + + if (pid == 0) { /* child - guestunmount */ + char fd_str[64]; + + close (pipefd[1]); + + snprintf (fd_str, sizeof fd_str, "%d", pipefd[0]); + + execlp ("./guestunmount", "guestunmount", "--fd", fd_str, "/", NULL); + perror ("execlp"); + _exit (EXIT_FAILURE); + } + + /* Parent continues. */ + close (pipefd[0]); + + /* Sleep a bit and test that the guestunmount process is still running. */ + sleep (2); + + r = waitpid (pid, &status, WNOHANG); + if (r == -1) { + perror ("waitpid"); + exit (EXIT_FAILURE); + } + if (r != 0) { + fprintf (stderr, "%s: test failed: guestunmount unexpectedly ", argv[0]); + display_exit_status (status, stderr); + fputc ('\n', stderr); + exit (EXIT_FAILURE); + } + + /* Close the write side of the pipe. This should cause guestunmount + * to exit. It should exit with status code _2_ because we gave it + * a mountpoint which isn't a FUSE mountpoint. + */ + close (pipefd[1]); + + r = waitpid (pid, &status, 0); + if (r == -1) { + perror ("waitpid"); + exit (EXIT_FAILURE); + } + if (!WIFEXITED (status) || WEXITSTATUS (status) != 2) { + fprintf (stderr, "%s: test failed: guestunmount didn't return status code 2; instead it ", argv[0]); + display_exit_status (status, stderr); + fputc ('\n', stderr); + exit (EXIT_FAILURE); + } + + exit (EXIT_SUCCESS); +} + +static void +display_exit_status (int status, FILE *fp) +{ + if (WIFEXITED (status)) + fprintf (fp, "exited with status code %d", WEXITSTATUS (status)); + else if (WIFSIGNALED (status)) { + fprintf (fp, "exited on signal %d", WTERMSIG (status)); + if (WCOREDUMP (status)) + fprintf (fp, " and dumped core"); + } + else if (WIFSTOPPED (status)) + fprintf (fp, "stopped on signal %d", WSTOPSIG (status)); + else + fprintf (fp, "<< unknown status %d >>", status); +} diff --git a/fuse/test-guestunmount-not-mounted.sh b/fuse/test-guestunmount-not-mounted.sh new file mode 100755 index 0000000..aa41a4c --- /dev/null +++ b/fuse/test-guestunmount-not-mounted.sh @@ -0,0 +1,50 @@ +#!/bin/bash - +# libguestfs +# Copyright (C) 2013 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. + +# https://bugzilla.redhat.com/show_bug.cgi?id=916780 +# Test that guestunmount returns the correct error code if +# there is no mounted FUSE filesystem. + +unset CDPATH +#set -e +#set -v + +if [ -n "$SKIP_TEST_GUESTUNMOUNT_NOT_MOUNTED_SH" ]; then + echo "$0: test skipped because environment variable is set." + exit 77 +fi + +if [ ! -w /dev/fuse ]; then + echo "SKIPPING guestunmount test, because there is no /dev/fuse." + exit 77 +fi + +# Not expecting cwd to be a FUSE mountpoint. +./guestunmount --quiet $(pwd) +r=$? +case $r in + 0) + echo "$0: failed: guestunmount should return exit code 2" ;; + 1) + echo "$0: failed: guestunmount failed to run, see errors above" ;; + 2) + # OK + ;; + *) + echo "$0: failed: guestunmount returned unknown error code $r" ;; +esac diff --git a/ocaml/t/guestfs_500_mount_local.ml b/ocaml/t/guestfs_500_mount_local.ml index 3047544..ca89a63 100644 --- a/ocaml/t/guestfs_500_mount_local.ml +++ b/ocaml/t/guestfs_500_mount_local.ml @@ -142,40 +142,9 @@ and test_mountpoint mp = done; if debug then eprintf "%s > unmounting filesystem\n%!" mp; - - unmount mp - -(* We may need to retry this a few times because of processes which - * run in the background jumping into mountpoints. Only display - * errors if it still fails after many retries. - *) -and unmount mp = - let logfile = sprintf "%s.fusermount.log" mp in - let unlink_logfile () = - try unlink logfile with Unix_error _ -> () - in - unlink_logfile (); - - let run_command () = - Sys.command (sprintf "fusermount -u %s >> %s 2>&1" - (Filename.quote mp) (Filename.quote logfile)) = 0 - in - - let rec loop tries = - if tries <= 5 then ( - if not (run_command ()) then ( - sleep 1; - loop (tries+1) - ) - ) else ( - ignore (Sys.command (sprintf "cat %s" (Filename.quote logfile))); - eprintf "fusermount: %s: failed, see earlier error messages\n" mp; - exit 1 - ) - in - loop 0; - - unlink_logfile () + ignore ( + Sys.command (sprintf "../fuse/guestunmount %s" (Filename.quote mp)) + ) let () = match Array.to_list Sys.argv with diff --git a/po-docs/ja/Makefile.am b/po-docs/ja/Makefile.am index d3413ec..748ae5e 100644 --- a/po-docs/ja/Makefile.am +++ b/po-docs/ja/Makefile.am @@ -41,6 +41,7 @@ MANPAGES = \ guestfs-testing.1 \ guestfsd.8 \ guestmount.1 \ + guestunmount.1 \ libguestfs-make-fixed-appliance.1 \ libguestfs-test-tool.1 \ virt-alignment-scan.1 \ diff --git a/po-docs/podfiles b/po-docs/podfiles index 74f722d..8f9179c 100644 --- a/po-docs/podfiles +++ b/po-docs/podfiles @@ -21,6 +21,7 @@ ../fish/virt-tar-out.pod ../format/virt-format.pod ../fuse/guestmount.pod +../fuse/guestunmount.pod ../guestfs-release-notes.pod ../inspector/virt-inspector.pod ../java/examples/guestfs-java.pod diff --git a/po-docs/uk/Makefile.am b/po-docs/uk/Makefile.am index d3413ec..748ae5e 100644 --- a/po-docs/uk/Makefile.am +++ b/po-docs/uk/Makefile.am @@ -41,6 +41,7 @@ MANPAGES = \ guestfs-testing.1 \ guestfsd.8 \ guestmount.1 \ + guestunmount.1 \ libguestfs-make-fixed-appliance.1 \ libguestfs-test-tool.1 \ virt-alignment-scan.1 \ diff --git a/po/POTFILES b/po/POTFILES index d746354..d8a9d9c 100644 --- a/po/POTFILES +++ b/po/POTFILES @@ -144,6 +144,8 @@ fish/tilde.c fish/time.c format/format.c fuse/guestmount.c +fuse/guestunmount.c +fuse/test-guestunmount-fd.c gobject/src/optargs-add_domain.c gobject/src/optargs-add_drive.c gobject/src/optargs-btrfs_filesystem_resize.c diff --git a/sysprep/Makefile.am b/sysprep/Makefile.am index e16a19d..de49d86 100644 --- a/sysprep/Makefile.am +++ b/sysprep/Makefile.am @@ -155,6 +155,7 @@ sysprep-operations.pod: virt-sysprep TESTS_ENVIRONMENT = \ abs_builddir=$(abs_builddir) \ abs_srcdir=$(abs_srcdir) \ + PATH=$(abs_top_builddir)/fuse:$(PATH) \ $(top_builddir)/run --test if ENABLE_APPLIANCE diff --git a/sysprep/sysprep_operation_script.ml b/sysprep/sysprep_operation_script.ml index a49bc3c..5c77146 100644 --- a/sysprep/sysprep_operation_script.ml +++ b/sysprep/sysprep_operation_script.ml @@ -77,7 +77,7 @@ let rec script_perform (g : Guestfs.guestfs) root = [] (* Run the scripts in the background and make sure they call - * fusermount afterwards. + * guestunmount afterwards. *) and run_scripts mp scripts = let sh = "/bin/bash" in @@ -89,19 +89,11 @@ cleanup () { status=$? cd / - count=10 - while ! fusermount -u %s >/dev/null 2>&1 && [ $count -gt 0 ]; do - sleep 1 - ((count--)) - done - if [ $count -eq 0 ]; then - echo \"fusermount: failed to unmount directory\" %s >&2 - exit 1 - fi + guestunmount %s ||: exit $status } trap cleanup INT TERM QUIT EXIT ERR\n" - (Filename.quote mp) (Filename.quote mp) ^ + (Filename.quote mp) ^ String.concat "\n" scripts in let args = [| sh; "-c"; cmd |] in diff --git a/tests/mount-local/test-parallel-mount-local.c b/tests/mount-local/test-parallel-mount-local.c index 0f2017e..74b04ac 100644 --- a/tests/mount-local/test-parallel-mount-local.c +++ b/tests/mount-local/test-parallel-mount-local.c @@ -59,9 +59,9 @@ static size_t nr_threads; static void *start_thread (void *) __attribute__((noreturn)); static void test_mountpoint (const char *mp); static void cleanup_thread_state (void); -static int unmount (const char *mp, unsigned flags); -#define UNMOUNT_SILENT 1 -#define UNMOUNT_RMDIR 2 +static int guestunmount (const char *mp, unsigned flags); +#define GUESTUNMOUNT_SILENT 1 +#define GUESTUNMOUNT_RMDIR 2 static volatile sig_atomic_t quit = 0; @@ -365,8 +365,8 @@ test_mountpoint (const char *mp) ret = EXIT_SUCCESS; error: ignore_value (chdir ("..")); - if (unmount (mp, 0) == -1) - error (EXIT_FAILURE, 0, "fusermount -u %s: failed, see earlier errors", mp); + if (guestunmount (mp, 0) == -1) + error (EXIT_FAILURE, 0, "guestunmount %s: failed, see earlier errors", mp); if (DEBUG) { printf ("%-8s > unmounted filesystem\n", mp); @@ -376,49 +376,28 @@ test_mountpoint (const char *mp) exit (ret); } -/* We may need to retry this a few times because of processes which - * run in the background jumping into mountpoints. Only display - * errors if it still fails after many retries. - */ static int -unmount (const char *mp, unsigned flags) +guestunmount (const char *mp, unsigned flags) { - char logfile[256]; char cmd[256]; - int tries = 5, status, r; + int status, r; - if (flags & UNMOUNT_RMDIR) { + if (flags & GUESTUNMOUNT_RMDIR) { r = rmdir (mp); if (r == 0 || (r == -1 && errno != EBUSY && errno != ENOTCONN)) return 0; } - snprintf (logfile, sizeof logfile, "%s.fusermount.tmp", mp); - unlink (logfile); - - snprintf (cmd, sizeof cmd, "fusermount -u %s >> %s 2>&1", mp, logfile); + snprintf (cmd, sizeof cmd, + "../../fuse/guestunmount%s %s", + (flags & GUESTUNMOUNT_SILENT) ? " --quiet" : "", mp); - while (tries > 0) { - status = system (cmd); - if (WIFEXITED (status) && WEXITSTATUS (status) == 0) - break; - sleep (1); - tries--; - } - - if (tries == 0) { /* Failed. */ - if (!(flags & UNMOUNT_SILENT)) { - fprintf (stderr, "fusermount -u %s: command failed:\n", mp); - snprintf (cmd, sizeof cmd, "cat %s", logfile); - ignore_value (system (cmd)); - } - unlink (logfile); + status = system (cmd); + if (!WIFEXITED (status) || + (WEXITSTATUS (status) != 0 && WEXITSTATUS (status) != 2)) return -1; - } - - unlink (logfile); - if (flags & UNMOUNT_RMDIR) { + if (flags & GUESTUNMOUNT_RMDIR) { if (rmdir (mp) == -1) return -1; } @@ -439,7 +418,7 @@ cleanup_thread_state (void) } if (threads[i].mp) { - unmount (threads[i].mp, UNMOUNT_SILENT|UNMOUNT_RMDIR); + guestunmount (threads[i].mp, GUESTUNMOUNT_SILENT|GUESTUNMOUNT_RMDIR); free (threads[i].mp); } } diff --git a/tests/selinux/run-test.pl b/tests/selinux/run-test.pl index 5258bb1..f55b1fb 100755 --- a/tests/selinux/run-test.pl +++ b/tests/selinux/run-test.pl @@ -175,28 +175,11 @@ sub run_fuse_tests } # Unmount the test directory. - unmount ($mpdir); - - exit ($errors == 0 ? 0 : 1); -} - -# Unmount the FUSE directory. We may need to retry this a few times. -sub unmount -{ - my $mpdir = shift; - my $retries = 5; - - while ($retries > 0) { - if (system ("fusermount", "-u", $mpdir) == 0) { - last; - } - sleep 1; - $retries--; - } - - if ($retries == 0) { + if (system ("../../fuse/guestunmount", $mpdir) != 0) { die "failed to unmount FUSE directory\n"; } + + exit ($errors == 0 ? 0 : 1); } # Test extended attributes, using the libguestfs API directly. -- 1.8.1.2