From: "Richard W.M. Jones" <rjones@redhat.com> See also: https://bugzilla.redhat.com/show_bug.cgi?id=835466#c9 --- fuse/guestmount.pod | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/fuse/guestmount.pod b/fuse/guestmount.pod index a0bc6f1..c37301c 100644 --- a/fuse/guestmount.pod +++ b/fuse/guestmount.pod @@ -106,6 +106,22 @@ namespace using the Linux-specific L<clone(2)>/L<unshare(2)> flag C<CLONE_NEWNS>. Unfortunately at the moment this requires root and we 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. + +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 + # immediately try to use 'disk.img' <-- UNSAFE + +The solution is to spin waiting for the guestmount process to exit. + =head1 OPTIONS =over 4 -- 1.7.10.4

From: "Richard W.M. Jones" <rjones@redhat.com> --- fuse/guestmount.c | 46 +++++++++++++++++++++++++++++++++++++++++++++- fuse/guestmount.pod | 8 +++++++- 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/fuse/guestmount.c b/fuse/guestmount.c index 0c2e2dc..fd31726 100644 --- a/fuse/guestmount.c +++ b/fuse/guestmount.c @@ -42,6 +42,8 @@ #include "guestmount.h" #include "options.h" +static int write_pid_file (const char *pid_file, pid_t pid); + #ifndef HAVE_FUSE_OPT_ADD_OPT_ESCAPED /* Copied from lib/fuse_opt.c and modified. * Copyright (C) 2001-2007 Miklos Szeredi <miklos@szeredi.hu> @@ -119,6 +121,7 @@ usage (int status) " -m|--mount dev[:mnt[:opts]] Mount dev on mnt (if omitted, /)\n" " -n|--no-sync Don't autosync\n" " -o|--option opt Pass extra option to FUSE\n" + " --pid-file filename Write PID to filename\n" " -r|--ro Mount read-only\n" " --selinux Enable SELinux support\n" " -v|--verbose Verbose messages\n" @@ -161,6 +164,7 @@ main (int argc, char *argv[]) { "mount", 1, 0, 'm' }, { "no-sync", 0, 0, 'n' }, { "option", 1, 0, 'o' }, + { "pid-file", 1, 0, 0 }, { "ro", 0, 0, 'r' }, { "rw", 0, 0, 'w' }, { "selinux", 0, 0, 0 }, @@ -184,6 +188,7 @@ main (int argc, char *argv[]) int dir_cache_timeout = -1; int do_fork = 1; char *fuse_options = NULL; + char *pid_file = NULL; struct guestfs_mount_local_argv optargs; @@ -227,6 +232,8 @@ main (int argc, char *argv[]) echo_keys = 1; } else if (STREQ (long_options[option_index].name, "live")) { live = 1; + } else if (STREQ (long_options[option_index].name, "pid-file")) { + pid_file = optarg; } else { fprintf (stderr, _("%s: unknown long option: %s (%d)\n"), program_name, long_options[option_index].name, option_index); @@ -406,8 +413,12 @@ main (int argc, char *argv[]) exit (EXIT_FAILURE); } - if (pid != 0) /* parent */ + if (pid != 0) { /* parent */ + if (write_pid_file (pid_file, pid) == -1) + _exit (EXIT_FAILURE); + _exit (EXIT_SUCCESS); + } /* Emulate what old fuse_daemonize used to do. */ if (setsid () == -1) { @@ -426,6 +437,11 @@ main (int argc, char *argv[]) close (fd); } } + else { + /* not forking, write PID file anyway */ + if (write_pid_file (pid_file, getpid ()) == -1) + exit (EXIT_FAILURE); + } /* Main loop. */ r = guestfs_mount_local_run (g); @@ -435,5 +451,33 @@ main (int argc, char *argv[]) r = -1; guestfs_close (g); + /* Don't delete PID file until the cleanup has been completed. */ + if (pid_file) + unlink (pid_file); + exit (r == 0 ? EXIT_SUCCESS : EXIT_FAILURE); } + +static int +write_pid_file (const char *pid_file, pid_t pid) +{ + FILE *fp; + + if (pid_file == NULL) + return 0; + + fp = fopen (pid_file, "w"); + if (fp == NULL) { + error: + perror (pid_file); + return -1; + } + + if (fprintf (fp, "%d\n", pid) == -1) + goto error; + + if (fclose (fp) == -1) + goto error; + + return 0; +} diff --git a/fuse/guestmount.pod b/fuse/guestmount.pod index d459045..fdcf133 100644 --- a/fuse/guestmount.pod +++ b/fuse/guestmount.pod @@ -120,7 +120,9 @@ condition: fusermount -u /mnt # immediately try to use 'disk.img' <-- UNSAFE -The solution is to spin waiting for the guestmount process to exit. +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. Note that if you use the C<guestfs_mount_local> API directly (see L<guestfs(3)/MOUNT LOCAL>) then it is much easier to write a safe, @@ -282,6 +284,10 @@ to the chosen values. =back +=item B<--pid-file filename> + +Write the PID of the guestmount worker process to C<filename>. + =item B<-r> =item B<--ro> -- 1.7.10.4