[PATCH 2/3] Implement mountee startup.

[Sergiu Ivanov]

* mount.c (mountee_node): New variable.
(mountee_root): Likewise.
(mountee_started): Likewise.
(start_mountee): New function (based on node_set_translator
in nsmux).
(setup_unionmount): New function.
* mount.h (mountee_root): New variable.
(mountee_started): Likewise.
(start_mountee): New function.
(setup_unionmount): New function.
* netfs.c (netfs_validate_stat): Start the mountee at the
first invocation.
---

Hello,

On Thu, Oct 29, 2009 at 06:37:54AM +0100, olafBuddenhagen@gmx.net wrote:
> On Mon, Aug 17, 2009 at 07:15:09PM +0300, Sergiu Ivanov wrote:
>
> > AIUI, this title line does say that some code for starting the mountee
> > is added.  Or do I understand something wrong?
> 
> The fact that you mention the specific detail "in a lazy fashion",
> creates the impression that this is the major point -- it sounds like
> the purpose of this patch is changing the code to use "lazy fashion"
> instead of some other approach used before... It's not at all clear that
> this patch adds the mountee starting code for the first time.

Hm, I can see your point.
 
> > I changed the title of this patch because the very first one -- ``Add
> > the code for starting up the mountee'' -- is too verbose (almost every
> > patch *adds* some code).
> 
> I don't think it is too verbose.
> 
> In either case, I don't quite understand that you try to make it less
> verbose by adding more detail?... :-)
> 
> "Add mountee startup" or "implement mountee startup" would be perfectly
> fine -- saying the same thing in less words. Or even just "start
> mountee", if you think the "add" is superfluous.

Ah :-) I've changed the name to ``Implement mountee startup''.  I
chose the titles for this series of patches quite a time ago, so,
frankly speaking, I've got so much used to them that I cannot analyze
them properly.
 
> > I won't submit patch with corrections you mention in this E-mail right
> > away, because the corrections are mainly about changing some comments
> > or strings and I think it will be harder for you to review the changes
> > if I post the whole patch again.
> 
> Well, I can't really give a final ACK without seeing the whole patch in
> its final form...

I'm sending the current version of the patch in this mail.  I didn't
intend to prevent you from reviewing the patch; rather I wanted to
save you from seeking a couple of minor modifications in the whole
patch.
 
> > Changed to: ``The mountee will be sitting on this node.  This node is
> > based on the netnode of the root node (it is essentially a clone of
> > the root node), so most RPCs on this node can be automatically carried
> > out correctly.  Note the we cannot set the mountee on the root node
> > directly, because in this case the mountee's filesystem will obscure
> > the filesystem published by unionfs.''
> 
> "most RPCs ont this node can be automatically carried out correctly" is
> way too vague... It's not ever clear what "correct" means in here, no
> what RPCs you mean.
> 
> I think you should say that the mountee is set on a (clone of) the
> unionfs root node, so that unionfs appears as the parent translator of
> the mountee. AIUI that's the idea behind it, right?

What exactly do you mean by ``parent translator''?  I must acknowledge
I haven't heard the term ``parent'' applied to translators (I can
attribute it to processes only).

Do you want to say that the goal of setting the mountee on a clone of
the root node is to make unionfs appear as the underlying translator
to the mountee?  If so, then yes, the idea behind cloning is really
this one and I will change the comment accordingly.
 
> > > Why are you passing O_READ, anyways?...
> > 
> > The flags which I pass to start_mountee are used in opening the port
> > to the root node of the mountee.  (I'm sure you've noticed this; I'm
> > just re-stating it to avoid ambiguities).  Inside unionfs, this port
> > is used for lookups *only*, so O_READ should be sufficient for any
> > internal unionfs needs.  Ports to files themselves are not proxied by
> > unionfs (as the comment reads), so the flags passed here don't
> > influence that case.
> 
> Hm, but wouldn't unionfs still need write permissions to the directories
> for adding new entries, when not in readonly mode?...

Well, obviously, O_READ permission on a directory is sufficient to
create files in it.  I got this suspicion from looking at unionfs
code: when unionfs looks up a directory, it creates a node for it.
When io_stat is invoked on this node, unionfs opens a port to the
corresponding directory with O_READ flags.  Subsequent requests to
create regular files (for instance) are carried out on this (O_READ)
port.

I wrote the following to test this supposition:

#define _GNU_SOURCE 1
#include <hurd.h>
#include <hurd/hurd_types.h>
#include <fcntl.h>

int
main (void)
{ 
  mach_port_t dir_p = file_name_lookup ("/tmp/", O_READ, 0);
  mach_port_t file_p = file_name_lookup_under (dir_p, "new-file",
                                               O_READ | O_CREAT, 0666);
  mach_port_deallocate (mach_task_self (), file_p);
  mach_port_deallocate (mach_task_self (), dir_p);

  return 0;
}

I built it with gcc -Wall -g .  After running ./a.out I get the new
file ``/tmp/new-file''.

I'm not sure whether this is a feature or a misbehaviour (frankly
speaking, I didn't expect this to work and was rather inclined to
consider that I had missed something).
 
> Also, if it's *always* O_READ, why do you pass it as a function
> parameter at all?

That's a good question :-) I guess I expected it to vary for some
reason.  I cannot see any such reasons now, so I removed the
parameter.

Regards,
scolobb

---
 mount.c |  142 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 mount.h |   17 ++++++++
 netfs.c |    7 +++
 3 files changed, 166 insertions(+), 0 deletions(-)

diff --git a/mount.c b/mount.c
index 7bc1fb8..a74b351 100644
--- a/mount.c
+++ b/mount.c
@@ -22,8 +22,150 @@
 
 #define _GNU_SOURCE
 
+#include <hurd/fsys.h>
+#include <fcntl.h>
+
 #include "mount.h"
+#include "lib.h"
 
 /* The command line for starting the mountee.  */
 char * mountee_argz;
 size_t mountee_argz_len;
+
+/* The node the mountee is sitting on.  */
+node_t * mountee_node;
+
+mach_port_t mountee_root;
+
+int mountee_started = 0;
+
+/* Starts the mountee (given by `argz` and `argz_len`), attaches it to
+   the node `np` and opens a port `port` to the mountee.  */
+error_t
+start_mountee (node_t * np, char * argz, size_t argz_len, mach_port_t * port)
+{
+  error_t err;
+  mach_port_t underlying_port;
+
+  mach_port_t mountee_control;
+
+  /* Identity information about the unionfs process (for
+     fsys_getroot).  */
+  uid_t * uids;
+  size_t nuids;
+
+  gid_t * gids;
+  size_t ngids;
+
+  /* The retry information returned by fsys_getroot.  */
+  string_t retry_name;
+  mach_port_t retry_port;
+
+  /* Fetch the effective UIDs of the unionfs process.  */
+  nuids = geteuids (0, 0);
+  if (nuids < 0)
+    return EPERM;
+  uids = alloca (nuids * sizeof (uid_t));
+
+  nuids = geteuids (nuids, uids);
+  assert (nuids > 0);
+
+  /* Fetch the effective GIDs of the unionfs process.  */
+  ngids = getgroups (0, 0);
+  if (ngids < 0)
+    return EPERM;
+  gids = alloca (ngids * sizeof (gid_t));
+
+  ngids = getgroups (ngids, gids);
+  assert (ngids > 0);
+
+  /* Opens the port on which to set the mountee.  */
+  error_t open_port (int flags, mach_port_t * underlying,
+                    mach_msg_type_name_t * underlying_type, task_t task,
+                    void *cookie)
+  {
+    err = 0;
+
+    /* The protid which will contain the port to the node on which the
+       mountee will be sitting.  */
+    struct protid * newpi;
+
+    struct iouser * unionfs_user;
+
+    /* Initialize `unionfs_user` with the effective UIDs and GIDs of
+       the unionfs process.  */
+    err = iohelp_create_complex_iouser (&unionfs_user, uids, nuids, gids, 
ngids);
+    if (err)
+      return err;
+
+    /* Create a port to node on which the mountee should sit (np).  */
+    newpi = netfs_make_protid
+      (netfs_make_peropen (np, flags, NULL), unionfs_user);
+    if (!newpi)
+      {
+       iohelp_free_iouser (unionfs_user);
+       return errno;
+      }
+
+    *underlying = underlying_port = ports_get_send_right (newpi);
+    *underlying_type = MACH_MSG_TYPE_COPY_SEND;
+
+    ports_port_deref (newpi);
+
+    return err;
+  }                            /*open_port */
+
+  /* Start the translator.  The value 60000 for the timeout is the one
+     found in settrans.  */
+  err = fshelp_start_translator (open_port, NULL, argz, argz, argz_len,
+                                60000, &mountee_control);
+  if (err)
+    return err;
+
+  /* Attach the mountee to the port opened in the previous call.  */
+  err = file_set_translator (underlying_port, 0, FS_TRANS_SET, 0, argz,
+                            argz_len, mountee_control, 
MACH_MSG_TYPE_COPY_SEND);
+  port_dealloc (underlying_port);
+  if (err)
+    return err;
+
+  /* Obtain the port to the root of the newly-set translator.
+
+     Note that the O_READ flag does not actually limit access to the
+     mountee's filesystem considerably.  Whenever a client looks up a
+     node which is not a directory, unionfs will give off a port to
+     the node itself, withouth proxying it.  Proxying happens only for
+     directory nodes.  */
+  err = fsys_getroot (mountee_control, MACH_PORT_NULL, MACH_MSG_TYPE_COPY_SEND,
+                     uids, nuids, gids, ngids, O_READ, &retry_port,
+                     retry_name, port);
+  return err;
+}                              /* start_mountee */
+
+/* Sets up a proxy node and sets the translator on it.  */
+error_t
+setup_unionmount (void)
+{
+  error_t err = 0;
+
+  /* The mountee will be sitting on this node.  This node is based on
+     the netnode of the root node (it is essentially a clone of the
+     root node), so most RPCs on this node can be automatically
+     carried out correctly.  Note the we cannot set the mountee on the
+     root node directly, because in this case the mountee's filesystem
+     will obscure the filesystem published by unionfs.  */
+  mountee_node = netfs_make_node (netfs_root_node->nn);
+  if (!mountee_node)
+    return ENOMEM;
+
+  /* Set the mountee on the new node.  */
+  err = start_mountee (mountee_node, mountee_argz, mountee_argz_len,
+                      &mountee_root);
+  if (err)
+    return err;
+
+  mountee_started = 1;
+
+  return 0;
+}                              /* setup_unionmount */
+
diff --git a/mount.h b/mount.h
index a7dd933..fd265f0 100644
--- a/mount.h
+++ b/mount.h
@@ -23,10 +23,27 @@
 #ifndef INCLUDED_MOUNT_H
 #define INCLUDED_MOUNT_H
 
+#include <error.h>
 #include <unistd.h>
+#include <hurd/hurd_types.h>
+
+#include "node.h"
 
 /* The command line for starting the mountee.  */
 extern char * mountee_argz;
 extern size_t mountee_argz_len;
 
+extern mach_port_t mountee_root;
+
+extern int mountee_started;
+
+/* Starts the mountee (given by `argz` and `argz_len`), attaches it to
+   the node `np` and opens a port `port` to the mountee.  */
+error_t
+start_mountee (node_t * np, char * argz, size_t argz_len, mach_port_t * port);
+
+/* Sets up a proxy node and sets the translator on it.  */
+error_t
+setup_unionmount (void);
+
 #endif /* not INCLUDED_MOUNT_H */
diff --git a/netfs.c b/netfs.c
index 3c23261..b9ea585 100644
--- a/netfs.c
+++ b/netfs.c
@@ -195,6 +195,13 @@ netfs_validate_stat (struct node *np, struct iouser *cred)
     }
   else 
     {
+      if (!mountee_started)
+       {
+         err = setup_unionmount ();
+         if (err)
+           error (EXIT_FAILURE, err, "failed to set up the mountee");
+       }
+
       _get_node_size (np, &np->nn_stat.st_size); 
     }
 
-- 
1.6.4.3