[PATCH] daemon: Run lsof when an umount command fails in umount_all call.
Useful for debugging unmount failures. Note that we include lsof in the appliance already. --- daemon/mount.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/daemon/mount.c b/daemon/mount.c index c5b7d89..e139482 100644 --- a/daemon/mount.c +++ b/daemon/mount.c @@ -424,6 +424,10 @@ do_umount_all (void) r = command (NULL, &err, str_umount, mounts.argv[i], NULL); if (r == -1) { reply_with_error ("umount: %s: %s", mounts.argv[i], err); + if (verbose) { + /* Try running lsof to see what is holding the mountpoint open. */ + command (NULL, NULL, "lsof", mounts.argv[i], NULL); + } free_stringslen (mounts.argv, mounts.size); return -1; } -- 2.4.3
On Thursday 23 July 2015 16:24:23 Richard W.M. Jones wrote:
Useful for debugging unmount failures. Note that we include lsof in the appliance already. --- daemon/mount.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/daemon/mount.c b/daemon/mount.c index c5b7d89..e139482 100644 --- a/daemon/mount.c +++ b/daemon/mount.c @@ -424,6 +424,10 @@ do_umount_all (void) r = command (NULL, &err, str_umount, mounts.argv[i], NULL); if (r == -1) { reply_with_error ("umount: %s: %s", mounts.argv[i], err); + if (verbose) { + /* Try running lsof to see what is holding the mountpoint open. */ + command (NULL, NULL, "lsof", mounts.argv[i], NULL); + } free_stringslen (mounts.argv, mounts.size); return -1; }
LGTM. Maybe worth being done also on single umount API? Thanks, -- Pino Toscano
On Thu, Jul 23, 2015 at 05:29:43PM +0200, Pino Toscano wrote:
On Thursday 23 July 2015 16:24:23 Richard W.M. Jones wrote:
Useful for debugging unmount failures. Note that we include lsof in the appliance already. --- daemon/mount.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/daemon/mount.c b/daemon/mount.c index c5b7d89..e139482 100644 --- a/daemon/mount.c +++ b/daemon/mount.c @@ -424,6 +424,10 @@ do_umount_all (void) r = command (NULL, &err, str_umount, mounts.argv[i], NULL); if (r == -1) { reply_with_error ("umount: %s: %s", mounts.argv[i], err); + if (verbose) { + /* Try running lsof to see what is holding the mountpoint open. */ + command (NULL, NULL, "lsof", mounts.argv[i], NULL); + } free_stringslen (mounts.argv, mounts.size); return -1; }
LGTM. Maybe worth being done also on single umount API?
Unfortunately there's no actual output from 'lsof'. I'm not sure if this is because I'm running lsof in the wrong way or (more likely) it's a race with some process holding open a file momentarily during the umount. So this patch is probably not right anyway. I wonder if we should use a lazy umount instead ... Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html
On Thu, Jul 23, 2015 at 04:36:50PM +0100, Richard W.M. Jones wrote:
On Thu, Jul 23, 2015 at 05:29:43PM +0200, Pino Toscano wrote:
On Thursday 23 July 2015 16:24:23 Richard W.M. Jones wrote:
Useful for debugging unmount failures. Note that we include lsof in the appliance already. --- daemon/mount.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/daemon/mount.c b/daemon/mount.c index c5b7d89..e139482 100644 --- a/daemon/mount.c +++ b/daemon/mount.c @@ -424,6 +424,10 @@ do_umount_all (void) r = command (NULL, &err, str_umount, mounts.argv[i], NULL); if (r == -1) { reply_with_error ("umount: %s: %s", mounts.argv[i], err); + if (verbose) { + /* Try running lsof to see what is holding the mountpoint open. */ + command (NULL, NULL, "lsof", mounts.argv[i], NULL); + } free_stringslen (mounts.argv, mounts.size); return -1; }
LGTM. Maybe worth being done also on single umount API?
Unfortunately there's no actual output from 'lsof'. I'm not sure if this is because I'm running lsof in the wrong way
I think it's because I'm running lsof the wrong way. This should be better: diff --git a/daemon/mount.c b/daemon/mount.c index c5b7d89..20a8d32 100644 --- a/daemon/mount.c +++ b/daemon/mount.c @@ -26,6 +26,8 @@ #include <sys/types.h> #include <mntent.h> +#include "ignore-value.h" + #include "daemon.h" #include "actions.h" @@ -421,6 +423,16 @@ do_umount_all (void) for (i = 0; i < mounts.size; ++i) { CLEANUP_FREE char *err = NULL; + if (verbose) { + CLEANUP_FREE char *cmd; + + if (asprintf_nowarn (&cmd, "lsof %Q", mounts.argv[i]) == -1) + perror ("asprintf"); + else + ignore_value (system (cmd)); + } + r = command (NULL, &err, str_umount, mounts.argv[i], NULL); if (r == -1) { reply_with_error ("umount: %s: %s", mounts.argv[i], err); -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top
participants (2)
-
Pino Toscano -
Richard W.M. Jones