Libguestfs - Jan 2014 - Re: [PATCH] fuse: In mount-local-run, test if root filesystem has been mounted (RHBZ#1057504).

It is never normally valid to use the mount-local* APIs when you
haven't mounted some filesystems in the libguestfs namespace.

If you try it, it results in some odd errors.  The mount-local-run
call is successful, but subsequent operations fail:

$ mkdir -p /tmp/mnt
$ guestfish -x -N fs mount-local /tmp/mnt : mount-local-run
libguestfs: error: lstat: lstat_stub: you must call 'mount' first to
mount the root filesystem
libguestfs: error: lstat: lstat_stub: you must call 'mount' first to
mount the root filesystem

With this commit, the mount-local-run call gives an error.

The error is implemented by calling guestfs_exists (g, "/") which has
the side effect of running the NEED_ROOT macro in the daemon.  (There
is no simple equivalent of NEED_ROOT on the library side.)
---
 src/fuse.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/fuse.c b/src/fuse.c
index dd4f139..c22cbba 100644
--- a/src/fuse.c
+++ b/src/fuse.c
@@ -1028,6 +1028,12 @@ guestfs__mount_local_run (guestfs_h *g)
     return -1;
   }
 
+  /* Test if root is mounted.  We do this by using a side-effect of
+   * guestfs_exists (which is that it calls NEED_ROOT).
+   */
+  if (guestfs_exists (g, "/") == -1)
+    return -1;
+
   debug (g, "%s: entering fuse_loop", __func__);
 
   /* Enter the main loop. */
-- 
1.8.4.2

On Friday 24 January 2014 10:22:58 Richard W.M. Jones
wrote:
> It is never normally valid to use the mount-local* APIs when you
> haven't mounted some filesystems in the libguestfs namespace.
> 
> If you try it, it results in some odd errors.  The mount-local-run
> call is successful, but subsequent operations fail:
> 
> $ mkdir -p /tmp/mnt
> $ guestfish -x -N fs mount-local /tmp/mnt : mount-local-run
> libguestfs: error: lstat: lstat_stub: you must call 'mount' first
to
> mount the root filesystem libguestfs: error: lstat: lstat_stub: you
> must call 'mount' first to mount the root filesystem
> 
> With this commit, the mount-local-run call gives an error.
> 
> The error is implemented by calling guestfs_exists (g, "/") which
has
> the side effect of running the NEED_ROOT macro in the daemon.  (There
> is no simple equivalent of NEED_ROOT on the library side.)
> ---
>  src/fuse.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/src/fuse.c b/src/fuse.c
> index dd4f139..c22cbba 100644
> --- a/src/fuse.c
> +++ b/src/fuse.c
> @@ -1028,6 +1028,12 @@ guestfs__mount_local_run (guestfs_h *g)
>      return -1;
>    }
> 
> +  /* Test if root is mounted.  We do this by using a side-effect of
> +   * guestfs_exists (which is that it calls NEED_ROOT).
> +   */
> +  if (guestfs_exists (g, "/") == -1)
> +    return -1;
> +
>    debug (g, "%s: entering fuse_loop", __func__);
> 
>    /* Enter the main loop. */
Sounds good (you are fast :) )
The only drawback is that the guestfish command above will leave the 
local FUSE mountpoint still mounted after the guestfish run.
Not sure whether it is worth doing anything automatic about that though, 
maybe just adding a
  error (g, _("remember to umount the mountpoint by invoking
               `guestumount` or using unmount-local"));
before the «return -1» in the patch above could help.

-- 
Pino Toscano

On Fri, Jan 24, 2014 at 01:07:59PM +0100, Pino Toscano
wrote:
> On Friday 24 January 2014 10:22:58 Richard W.M. Jones wrote:
> > It is never normally valid to use the mount-local* APIs when you
> > haven't mounted some filesystems in the libguestfs namespace.
> > 
> > If you try it, it results in some odd errors.  The mount-local-run
> > call is successful, but subsequent operations fail:
> > 
> > $ mkdir -p /tmp/mnt
> > $ guestfish -x -N fs mount-local /tmp/mnt : mount-local-run
> > libguestfs: error: lstat: lstat_stub: you must call 'mount'
first to
> > mount the root filesystem libguestfs: error: lstat: lstat_stub: you
> > must call 'mount' first to mount the root filesystem
> > 
> > With this commit, the mount-local-run call gives an error.
> > 
> > The error is implemented by calling guestfs_exists (g, "/")
which has
> > the side effect of running the NEED_ROOT macro in the daemon.  (There
> > is no simple equivalent of NEED_ROOT on the library side.)
> > ---
> >  src/fuse.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/src/fuse.c b/src/fuse.c
> > index dd4f139..c22cbba 100644
> > --- a/src/fuse.c
> > +++ b/src/fuse.c
> > @@ -1028,6 +1028,12 @@ guestfs__mount_local_run (guestfs_h *g)
> >      return -1;
> >    }
> > 
> > +  /* Test if root is mounted.  We do this by using a side-effect of
> > +   * guestfs_exists (which is that it calls NEED_ROOT).
> > +   */
> > +  if (guestfs_exists (g, "/") == -1)
> > +    return -1;
> > +
> >    debug (g, "%s: entering fuse_loop", __func__);
> > 
> >    /* Enter the main loop. */
> 
> Sounds good (you are fast :) )
> The only drawback is that the guestfish command above will leave the 
> local FUSE mountpoint still mounted after the guestfish run.
> Not sure whether it is worth doing anything automatic about that though, 
> maybe just adding a
>   error (g, _("remember to umount the mountpoint by invoking
>                `guestumount` or using unmount-local"));
> before the «return -1» in the patch above could help.
Yes, I noticed this too.  I don't think unmounting it is a good
idea, but I've improved the error message.

Thanks,

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming blog: http://rwmj.wordpress.com
Fedora now supports 80 OCaml packages (the OPEN alternative to F#)