Libguestfs - Jul 2015 - Re: [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